Hi,

On 7/6/22 3:30 PM, Drouvot, Bertrand wrote:
Hi,

On 10/28/21 11:07 PM, Andres Freund wrote:
Hi,

On 2021-10-28 16:24:22 -0400, Robert Haas wrote:
On Wed, Oct 27, 2021 at 2:56 AM Drouvot, Bertrand <bdrou...@amazon.com> wrote:
So you have in mind to check for XLogLogicalInfoActive() first, and if true, then open the relation and call
RelationIsAccessibleInLogicalDecoding()?
I think 0001 is utterly unacceptable. We cannot add calls to
table_open() in low-level functions like this. Suppose for example
that _bt_getbuf() calls _bt_log_reuse_page() which with 0001 applied
would call get_rel_logical_catalog(). _bt_getbuf() will have acquired
a buffer lock on the page. The idea that it's safe to call
table_open() while holding a buffer lock cannot be taken seriously.
Yes - that's pretty clearly a deadlock hazard. It shouldn't too hard to fix, I
think. Possibly a bit more verbose than nice, but...

Alternatively we could propagate the information whether a relcache entry is for a catalog from the table to the index. Then we'd not need to change the
btree code to pass the table down.

Looking closer at RelationIsAccessibleInLogicalDecoding() It seems to me that the missing part to be able to tell whether or not an index is for a catalog is the rd_options->user_catalog_table value of its related heap relation.

Then, a way to achieve that could be to:

- Add to Relation a new "heap_rd_options" representing the rd_options of the related heap relation when appropriate

- Trigger the related indexes relcache invalidations when an ATExecSetRelOptions() is triggered on a heap relation

- Write an equivalent of RelationIsUsedAsCatalogTable() for indexes that would make use of the heap_rd_options instead

Does that sound like a valid option to you or do you have another idea in mind to propagate the information whether a relcache entry is for a catalog from the table to the index?


I ended up with the attached proposal to propagate the catalog information to the indexes.

The attached adds a new field "isusercatalog" in pg_index to indicate whether or not the index is linked to a table that has the storage parameter user_catalog_table set to true.

Then it defines new macros, including "IndexIsAccessibleInLogicalDecoding" making use of this new field.

This new macro replaces get_rel_logical_catalog() that was part of the previous patch version.

What do you think about this approach and the attached?

If that sounds reasonable, then I'll add tap tests for it and try to improve the way isusercatalog is propagated to the index(es) in case a reset is done on user_catalog_table on the table (currently in this POC patch, it's hardcoded to "false" which is the default value for user_catalog_table in boolRelOpts[]) (A better approach would be probably to retrieve the value from the table once the reset is done and then propagate it to the index(es).)

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 0dc838a0fdb0f8daccc398bea4f11152b68e0c3a Mon Sep 17 00:00:00 2001
From: bdrouvotAWS <bdrou...@amazon.com>
Date: Fri, 30 Sep 2022 09:28:09 +0000
Subject: [PATCH v26] Add info in WAL records in preparation for logical slot
 conflict handling.

When a WAL replay on standby indicates that a catalog table tuple is
to be deleted by an xid that is greater than a logical slot's
catalog_xmin, then that means the slot's catalog_xmin conflicts with
the xid, and we need to handle the conflict.  While subsequent commits
will do the actual conflict handling, this commit adds a new field
onCatalogTable in such WAL records, that is true for catalog tables,
so as to arrange for conflict handling.

Author: Andres Freund (in an older version), Amit Khandekar, Bertrand
Drouvot
Reviewed-By: Bertrand Drouvot, Andres Freund, Robert Haas, Fabrizio de
Royes Mello
---
 doc/src/sgml/catalogs.sgml              | 11 +++++
 src/backend/access/common/reloptions.c  |  3 +-
 src/backend/access/gist/gistxlog.c      |  1 +
 src/backend/access/hash/hashinsert.c    |  1 +
 src/backend/access/heap/heapam.c        |  4 +-
 src/backend/access/heap/pruneheap.c     |  1 +
 src/backend/access/heap/visibilitymap.c |  3 +-
 src/backend/access/nbtree/nbtpage.c     |  3 ++
 src/backend/access/spgist/spgvacuum.c   |  1 +
 src/backend/catalog/index.c             | 14 ++++--
 src/backend/commands/tablecmds.c        | 57 +++++++++++++++++++++++++
 src/include/access/gistxlog.h           |  2 +
 src/include/access/hash_xlog.h          |  1 +
 src/include/access/heapam_xlog.h        |  5 ++-
 src/include/access/nbtxlog.h            |  2 +
 src/include/access/spgxlog.h            |  1 +
 src/include/catalog/pg_index.h          |  2 +
 src/include/utils/rel.h                 | 33 ++++++++++++++
 18 files changed, 137 insertions(+), 8 deletions(-)
   8.3% doc/src/sgml/
   9.5% src/backend/access/heap/
   8.1% src/backend/access/
   7.7% src/backend/catalog/
  31.5% src/backend/commands/
   6.4% src/include/access/
  26.5% src/include/utils/

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 00f833d210..2a63ab0ea3 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -4426,6 +4426,17 @@ SCRAM-SHA-256$<replaceable>&lt;iteration 
count&gt;</replaceable>:<replaceable>&l
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>indisusercatalog</structfield> <type>bool</type>
+      </para>
+      <para>
+       If true, the index is linked to a table that is declared as an 
additional
+       catalog table for purposes of logical replication (means has <link 
linkend="sql-createtable"><literal>user_catalog_table</literal></link>)
+       set to true.
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>indisreplident</structfield> <type>bool</type>
diff --git a/src/backend/access/common/reloptions.c 
b/src/backend/access/common/reloptions.c
index 6458a9c276..44dc140440 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -120,7 +120,8 @@ static relopt_bool boolRelOpts[] =
                        RELOPT_KIND_HEAP,
                        AccessExclusiveLock
                },
-               false
+               false                                   /* Change 
catalog_table_val in
+                                                                * 
ATExecSetRelOptions accordingly */
        },
        {
                {
diff --git a/src/backend/access/gist/gistxlog.c 
b/src/backend/access/gist/gistxlog.c
index 998befd2cb..96107b2124 100644
--- a/src/backend/access/gist/gistxlog.c
+++ b/src/backend/access/gist/gistxlog.c
@@ -608,6 +608,7 @@ gistXLogPageReuse(Relation rel, BlockNumber blkno, 
FullTransactionId latestRemov
         */
 
        /* XLOG stuff */
+       xlrec_reuse.onCatalogTable = IndexIsAccessibleInLogicalDecoding(rel);
        xlrec_reuse.locator = rel->rd_locator;
        xlrec_reuse.block = blkno;
        xlrec_reuse.latestRemovedFullXid = latestRemovedXid;
diff --git a/src/backend/access/hash/hashinsert.c 
b/src/backend/access/hash/hashinsert.c
index 4f2fecb908..1c586b13f5 100644
--- a/src/backend/access/hash/hashinsert.c
+++ b/src/backend/access/hash/hashinsert.c
@@ -399,6 +399,7 @@ _hash_vacuum_one_page(Relation rel, Relation hrel, Buffer 
metabuf, Buffer buf)
                        xl_hash_vacuum_one_page xlrec;
                        XLogRecPtr      recptr;
 
+                       xlrec.onCatalogTable = 
RelationIsAccessibleInLogicalDecoding(hrel);
                        xlrec.latestRemovedXid = latestRemovedXid;
                        xlrec.ntuples = ndeletable;
 
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 75b214824d..1529bf1db0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8167,6 +8167,7 @@ log_heap_freeze(Relation reln, Buffer buffer, 
TransactionId cutoff_xid,
        /* nor when there are no tuples to freeze */
        Assert(ntuples > 0);
 
+       xlrec.onCatalogTable = RelationIsAccessibleInLogicalDecoding(reln);
        xlrec.cutoff_xid = cutoff_xid;
        xlrec.ntuples = ntuples;
 
@@ -8197,7 +8198,7 @@ log_heap_freeze(Relation reln, Buffer buffer, 
TransactionId cutoff_xid,
  * heap_buffer, if necessary.
  */
 XLogRecPtr
-log_heap_visible(RelFileLocator rlocator, Buffer heap_buffer, Buffer vm_buffer,
+log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer,
                                 TransactionId cutoff_xid, uint8 vmflags)
 {
        xl_heap_visible xlrec;
@@ -8207,6 +8208,7 @@ log_heap_visible(RelFileLocator rlocator, Buffer 
heap_buffer, Buffer vm_buffer,
        Assert(BufferIsValid(heap_buffer));
        Assert(BufferIsValid(vm_buffer));
 
+       xlrec.onCatalogTable = RelationIsAccessibleInLogicalDecoding(rel);
        xlrec.cutoff_xid = cutoff_xid;
        xlrec.flags = vmflags;
        XLogBeginInsert();
diff --git a/src/backend/access/heap/pruneheap.c 
b/src/backend/access/heap/pruneheap.c
index 9f43bbe25f..c3f9e62cc5 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -421,6 +421,7 @@ heap_page_prune(Relation relation, Buffer buffer,
                        xlrec.latestRemovedXid = prstate.latestRemovedXid;
                        xlrec.nredirected = prstate.nredirected;
                        xlrec.ndead = prstate.ndead;
+                       xlrec.onCatalogTable = 
RelationIsAccessibleInLogicalDecoding(relation);
 
                        XLogBeginInsert();
                        XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
diff --git a/src/backend/access/heap/visibilitymap.c 
b/src/backend/access/heap/visibilitymap.c
index d62761728b..d9abcccd68 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -283,8 +283,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer 
heapBuf,
                        if (XLogRecPtrIsInvalid(recptr))
                        {
                                Assert(!InRecovery);
-                               recptr = log_heap_visible(rel->rd_locator, 
heapBuf, vmBuf,
-                                                                               
  cutoff_xid, flags);
+                               recptr = log_heap_visible(rel, heapBuf, vmBuf, 
cutoff_xid, flags);
 
                                /*
                                 * If data checksums are enabled (or 
wal_log_hints=on), we
diff --git a/src/backend/access/nbtree/nbtpage.c 
b/src/backend/access/nbtree/nbtpage.c
index 8b96708b3e..8c18ff895a 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -836,6 +836,7 @@ _bt_log_reuse_page(Relation rel, BlockNumber blkno, 
FullTransactionId safexid)
         */
 
        /* XLOG stuff */
+       xlrec_reuse.onCatalogTable = IndexIsAccessibleInLogicalDecoding(rel);
        xlrec_reuse.locator = rel->rd_locator;
        xlrec_reuse.block = blkno;
        xlrec_reuse.latestRemovedFullXid = safexid;
@@ -1357,6 +1358,8 @@ _bt_delitems_delete(Relation rel, Buffer buf, 
TransactionId latestRemovedXid,
                XLogRecPtr      recptr;
                xl_btree_delete xlrec_delete;
 
+               xlrec_delete.onCatalogTable =
+                       IndexIsAccessibleInLogicalDecoding(rel);
                xlrec_delete.latestRemovedXid = latestRemovedXid;
                xlrec_delete.ndeleted = ndeletable;
                xlrec_delete.nupdated = nupdatable;
diff --git a/src/backend/access/spgist/spgvacuum.c 
b/src/backend/access/spgist/spgvacuum.c
index 0049630532..1fda720a77 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -503,6 +503,7 @@ vacuumRedirectAndPlaceholder(Relation index, Buffer buffer)
        spgxlogVacuumRedirect xlrec;
        GlobalVisState *vistest;
 
+       xlrec.onCatalogTable = IndexIsAccessibleInLogicalDecoding(index);
        xlrec.nToPlaceholder = 0;
        xlrec.newestRedirectXid = InvalidTransactionId;
 
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 61f1d3926a..f6b2c9ac71 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -123,7 +123,8 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
                                                                bool 
isexclusion,
                                                                bool immediate,
                                                                bool isvalid,
-                                                               bool isready);
+                                                               bool isready,
+                                                               bool 
is_user_catalog);
 static void index_update_stats(Relation rel,
                                                           bool hasindex,
                                                           double reltuples);
@@ -545,7 +546,8 @@ UpdateIndexRelation(Oid indexoid,
                                        bool isexclusion,
                                        bool immediate,
                                        bool isvalid,
-                                       bool isready)
+                                       bool isready,
+                                       bool is_user_catalog)
 {
        int2vector *indkey;
        oidvector  *indcollation;
@@ -622,6 +624,7 @@ UpdateIndexRelation(Oid indexoid,
        values[Anum_pg_index_indcheckxmin - 1] = BoolGetDatum(false);
        values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
        values[Anum_pg_index_indislive - 1] = BoolGetDatum(true);
+       values[Anum_pg_index_indisusercatalog - 1] = 
BoolGetDatum(is_user_catalog);
        values[Anum_pg_index_indisreplident - 1] = BoolGetDatum(false);
        values[Anum_pg_index_indkey - 1] = PointerGetDatum(indkey);
        values[Anum_pg_index_indcollation - 1] = PointerGetDatum(indcollation);
@@ -735,6 +738,7 @@ index_create(Relation heapRelation,
        TransactionId relfrozenxid;
        MultiXactId relminmxid;
        bool            create_storage = !RelFileNumberIsValid(relFileNumber);
+       bool            isusercatalog = false;
 
        /* constraint flags can only be set when a constraint is requested */
        Assert((constr_flags == 0) ||
@@ -1014,13 +1018,17 @@ index_create(Relation heapRelation,
         *        (Or, could define a rule to maintain the predicate) --Nels, 
Feb '92
         * ----------------
         */
+       if (heapRelation->rd_options)
+               isusercatalog = ((StdRdOptions *) 
(heapRelation)->rd_options)->user_catalog_table;
+
        UpdateIndexRelation(indexRelationId, heapRelationId, parentIndexRelid,
                                                indexInfo,
                                                collationObjectId, 
classObjectId, coloptions,
                                                isprimary, is_exclusion,
                                                (constr_flags & 
INDEX_CONSTR_CREATE_DEFERRABLE) == 0,
                                                !concurrent && !invalid,
-                                               !concurrent);
+                                               !concurrent,
+                                               isusercatalog);
 
        /*
         * Register relcache invalidation on the indexes' heap relation, to
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7d8a75d23c..593a7c085f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14136,6 +14136,10 @@ ATPrepSetTableSpace(AlteredTableInfo *tab, Relation 
rel, const char *tablespacen
 
 /*
  * Set, reset, or replace reloptions.
+ *
+ * The catalog_table_val value has to match the user_catalog_table value
+ * defined in boolRelOpts[] in reloptions.c. It's indeed used as the default
+ * value to be propagated to the indexes in case of reset.
  */
 static void
 ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
@@ -14151,6 +14155,10 @@ ATExecSetRelOptions(Relation rel, List *defList, 
AlterTableType operation,
        Datum           repl_val[Natts_pg_class];
        bool            repl_null[Natts_pg_class];
        bool            repl_repl[Natts_pg_class];
+       ListCell   *cell;
+       List       *rel_options;
+       bool            catalog_table_val = false;
+       bool            catalog_table = false;
        static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
 
        if (defList == NIL && operation != AT_ReplaceRelOptions)
@@ -14245,6 +14253,20 @@ ATExecSetRelOptions(Relation rel, List *defList, 
AlterTableType operation,
                }
        }
 
+       /* If user_catalog_table is part of the new options, record its new 
value */
+       rel_options = untransformRelOptions(newOptions);
+
+       foreach(cell, rel_options)
+       {
+               DefElem    *defel = (DefElem *) lfirst(cell);
+
+               if (strcmp(defel->defname, "user_catalog_table") == 0)
+               {
+                       catalog_table = true;
+                       catalog_table_val = defGetBoolean(defel);
+               }
+       }
+
        /*
         * All we need do here is update the pg_class row; the new options will 
be
         * propagated into relcaches during post-commit cache inval.
@@ -14271,6 +14293,41 @@ ATExecSetRelOptions(Relation rel, List *defList, 
AlterTableType operation,
 
        ReleaseSysCache(tuple);
 
+       /* Update the indexes if there is a need to */
+       if (catalog_table || operation == AT_ResetRelOptions)
+       {
+               Relation        pg_index;
+               HeapTuple       pg_index_tuple;
+               Form_pg_index pg_index_form;
+               ListCell   *index;
+
+               pg_index = table_open(IndexRelationId, RowExclusiveLock);
+
+               foreach(index, RelationGetIndexList(rel))
+               {
+                       Oid                     thisIndexOid = 
lfirst_oid(index);
+
+                       pg_index_tuple = SearchSysCacheCopy1(INDEXRELID,
+                                                                               
                 ObjectIdGetDatum(thisIndexOid));
+                       if (!HeapTupleIsValid(pg_index_tuple))
+                               elog(ERROR, "cache lookup failed for index %u", 
thisIndexOid);
+                       pg_index_form = (Form_pg_index) 
GETSTRUCT(pg_index_tuple);
+
+                       /* Modify the index only if user_catalog_table differ */
+                       if (catalog_table_val != 
pg_index_form->indisusercatalog)
+                       {
+                               pg_index_form->indisusercatalog = 
catalog_table_val;
+                               CatalogTupleUpdate(pg_index, 
&pg_index_tuple->t_self, pg_index_tuple);
+                               InvokeObjectPostAlterHookArg(IndexRelationId, 
thisIndexOid, 0,
+                                                                               
         InvalidOid, true);
+                       }
+
+                       heap_freetuple(pg_index_tuple);
+               }
+
+               table_close(pg_index, RowExclusiveLock);
+       }
+
        /* repeat the whole exercise for the toast table, if there's one */
        if (OidIsValid(rel->rd_rel->reltoastrelid))
        {
diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h
index 9bbe4c2622..c46c4728e1 100644
--- a/src/include/access/gistxlog.h
+++ b/src/include/access/gistxlog.h
@@ -49,6 +49,7 @@ typedef struct gistxlogPageUpdate
  */
 typedef struct gistxlogDelete
 {
+       bool            onCatalogTable;
        TransactionId latestRemovedXid;
        uint16          ntodelete;              /* number of deleted offsets */
 
@@ -97,6 +98,7 @@ typedef struct gistxlogPageDelete
  */
 typedef struct gistxlogPageReuse
 {
+       bool            onCatalogTable;
        RelFileLocator locator;
        BlockNumber block;
        FullTransactionId latestRemovedFullXid;
diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h
index 59230706bb..9dda97a8d7 100644
--- a/src/include/access/hash_xlog.h
+++ b/src/include/access/hash_xlog.h
@@ -250,6 +250,7 @@ typedef struct xl_hash_init_bitmap_page
  */
 typedef struct xl_hash_vacuum_one_page
 {
+       bool            onCatalogTable;
        TransactionId latestRemovedXid;
        int                     ntuples;
 
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 34220d93cf..a091845647 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -242,6 +242,7 @@ typedef struct xl_heap_update
  */
 typedef struct xl_heap_prune
 {
+       bool            onCatalogTable;
        TransactionId latestRemovedXid;
        uint16          nredirected;
        uint16          ndead;
@@ -338,6 +339,7 @@ typedef struct xl_heap_freeze_tuple
  */
 typedef struct xl_heap_freeze_page
 {
+       bool            onCatalogTable;
        TransactionId cutoff_xid;
        uint16          ntuples;
 } xl_heap_freeze_page;
@@ -352,6 +354,7 @@ typedef struct xl_heap_freeze_page
  */
 typedef struct xl_heap_visible
 {
+       bool            onCatalogTable;
        TransactionId cutoff_xid;
        uint8           flags;
 } xl_heap_visible;
@@ -415,7 +418,7 @@ extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
                                                                          
MultiXactId *relminmxid_out);
 extern void heap_execute_freeze_tuple(HeapTupleHeader tuple,
                                                                          
xl_heap_freeze_tuple *frz);
-extern XLogRecPtr log_heap_visible(RelFileLocator rlocator, Buffer heap_buffer,
+extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer,
                                                                   Buffer 
vm_buffer, TransactionId cutoff_xid, uint8 vmflags);
 
 #endif                                                 /* HEAPAM_XLOG_H */
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index dd504d1885..2a00e05560 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -185,6 +185,7 @@ typedef struct xl_btree_dedup
  */
 typedef struct xl_btree_reuse_page
 {
+       bool            onCatalogTable;
        RelFileLocator locator;
        BlockNumber block;
        FullTransactionId latestRemovedFullXid;
@@ -232,6 +233,7 @@ typedef struct xl_btree_vacuum
 
 typedef struct xl_btree_delete
 {
+       bool            onCatalogTable;
        TransactionId latestRemovedXid;
        uint16          ndeleted;
        uint16          nupdated;
diff --git a/src/include/access/spgxlog.h b/src/include/access/spgxlog.h
index 930ffdd4f7..4808303585 100644
--- a/src/include/access/spgxlog.h
+++ b/src/include/access/spgxlog.h
@@ -237,6 +237,7 @@ typedef struct spgxlogVacuumRoot
 
 typedef struct spgxlogVacuumRedirect
 {
+       bool            onCatalogTable;
        uint16          nToPlaceholder; /* number of redirects to make 
placeholders */
        OffsetNumber firstPlaceholder;  /* first placeholder tuple to remove */
        TransactionId newestRedirectXid;        /* newest XID of removed 
redirects */
diff --git a/src/include/catalog/pg_index.h b/src/include/catalog/pg_index.h
index f853846ee1..dd16431378 100644
--- a/src/include/catalog/pg_index.h
+++ b/src/include/catalog/pg_index.h
@@ -43,6 +43,8 @@ CATALOG(pg_index,2610,IndexRelationId) BKI_SCHEMA_MACRO
        bool            indcheckxmin;   /* must we wait for xmin to be old? */
        bool            indisready;             /* is this index ready for 
inserts? */
        bool            indislive;              /* is this index alive at all? 
*/
+       bool            indisusercatalog;       /* is this index linked to a 
user catalog
+                                                                        * 
relation? */
        bool            indisreplident; /* is this index the identity for 
replication? */
 
        /* variable-length fields start here, but we allow direct access to 
indkey */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 7dc401cf0d..6f626cc12d 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -27,6 +27,7 @@
 #include "storage/smgr.h"
 #include "utils/relcache.h"
 #include "utils/reltrigger.h"
+#include "catalog/catalog.h"
 
 
 /*
@@ -378,6 +379,9 @@ typedef struct StdRdOptions
  * RelationIsUsedAsCatalogTable
  *             Returns whether the relation should be treated as a catalog 
table
  *             from the pov of logical decoding.  Note multiple eval of 
argument!
+ *             This definition should not invoke anything that performs catalog
+ *             access; otherwise it may cause infinite recursion. Check the 
comments
+ *             in RelationIsAccessibleInLogicalDecoding() for details.
  */
 #define RelationIsUsedAsCatalogTable(relation) \
        ((relation)->rd_options && \
@@ -679,12 +683,41 @@ RelationGetSmgr(Relation rel)
  * RelationIsAccessibleInLogicalDecoding
  *             True if we need to log enough information to have access via
  *             decoding snapshot.
+ *             This definition should not invoke anything that performs catalog
+ *             access. Otherwise, e.g. logging a WAL entry for catalog 
relation may
+ *             invoke this function, which will in turn do catalog access, 
which may
+ *             in turn cause another similar WAL entry to be logged, leading to
+ *             infinite recursion.
  */
 #define RelationIsAccessibleInLogicalDecoding(relation) \
        (XLogLogicalInfoActive() && \
         RelationNeedsWAL(relation) && \
         (IsCatalogRelation(relation) || 
RelationIsUsedAsCatalogTable(relation)))
 
+/*
+ * IndexIsUserCatalog
+ *             True if index is linked to a user catalog relation.
+ */
+#define IndexIsUserCatalog(relation)                                           
                                        \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX),               
                \
+        (relation)->rd_index->indisusercatalog)
+
+/*
+ * IndexIsAccessibleInLogicalDecoding
+ *             True if we need to log enough information to have access via
+ *             decoding snapshot.
+ *             This definition should not invoke anything that performs catalog
+ *             access. Otherwise, e.g. logging a WAL entry for catalog 
relation may
+ *             invoke this function, which will in turn do catalog access, 
which may
+ *             in turn cause another similar WAL entry to be logged, leading to
+ *             infinite recursion.
+ */
+#define IndexIsAccessibleInLogicalDecoding(relation) \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX), \
+        XLogLogicalInfoActive() && \
+        RelationNeedsWAL(relation) && \
+        (IsCatalogRelation(relation) || IndexIsUserCatalog(relation)))
+
 /*
  * RelationIsLogicallyLogged
  *             True if we need to log enough information to extract the data 
from the
-- 
2.34.1

Reply via email to