Remove unnecessary checks for indexes for REPLICA IDENTITY FULL tables.
authorMasahiko Sawada
Tue, 25 Jul 2023 06:09:31 +0000 (15:09 +0900)
committerMasahiko Sawada
Tue, 25 Jul 2023 06:09:31 +0000 (15:09 +0900)
Previously, when selecting an usable index for update/delete for the
REPLICA IDENTITY FULL table, in IsIndexOnlyExpression(), we used to
check if all index fields are not expressions. However, it was not
necessary, because it is enough to check if only the leftmost index
field is not an expression (and references the remote table column)
and this check has already been done by
RemoteRelContainsLeftMostColumnOnIdx().

This commit removes IsIndexOnlyExpression() and
RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable
indexes for REPLICA IDENTITY FULL tables are now performed by
IsIndexUsableForReplicaIdentityFull().

Backpatch this to remain the code consistent.

Reported-by: Peter Smith
Reviewed-by: Amit Kapila, Önder Kalacı
Discussion: https://postgr.es/m/CAHut%2BPsGRE5WSsY0jcLHJEoA17MrbP9yy8FxdjC_ZOAACxbt%2BQ%40mail.gmail.com
Backpatch-through: 16

src/backend/executor/execReplication.c
src/backend/replication/logical/relation.c
src/backend/replication/logical/worker.c
src/include/replication/logicalrelation.h

index af093428813b30473c5c38f99a0e1d9821394f90..25d2868744e92d4647d2da44a4c19784317eafe1 100644 (file)
@@ -175,16 +175,7 @@ retry:
        if (!isIdxSafeToSkipDuplicates)
        {
            if (eq == NULL)
-           {
-#ifdef USE_ASSERT_CHECKING
-               /* apply assertions only once for the input idxoid */
-               IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-               Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
                eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-           }
 
            if (!tuples_equal(outslot, searchslot, eq))
                continue;
index c545b90636f8c4daac2f1081168dab9278bc2da9..6edbd5643ae0296f038f0da20a7076dd9f64091c 100644 (file)
@@ -731,71 +731,9 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
    return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- *     CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- *      CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-   for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-   {
-       AttrNumber  attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-       if (AttributeNumberIsValid(attnum))
-           return false;
-   }
-
-   return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-   AttrNumber  keycol;
-
-   Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-   keycol = indexInfo->ii_IndexAttrNumbers[0];
-   if (!AttributeNumberIsValid(keycol))
-       return false;
-
-   if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-       return false;
-
-   return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree, non-partial, and the leftmost
- * field must be a column (not an expression) that references the remote
- * relation column. These limitations help to keep the index scan similar
- * to PK/RI index scans.
- *
- * Note that the limitations of index scans for replica identity full only
- * adheres to a subset of the limitations of PK/RI. For example, we support
- * columns that are marked as [NULL] or we are not interested in the [NOT
- * DEFERRABLE] aspect of constraints here. It works for us because we always
- * compare the tuples for non-PK/RI index scans. See
- * RelationFindReplTupleByIndex().
- *
- * XXX: There are no fundamental problems for supporting non-btree indexes.
- * We mostly need to relax the limitations in RelationFindReplTupleByIndex().
- * For partial indexes, the required changes are likely to be larger. If
- * none of the tuples satisfy the expression for the index scan, we fall-back
- * to sequential execution, which might not be a good idea in some cases.
+ * the relation.
  *
  * We expect to call this function when REPLICA IDENTITY FULL is defined for
  * the remote relation.
@@ -812,19 +750,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
    {
        Oid         idxoid = lfirst_oid(lc);
        bool        isUsableIdx;
-       bool        containsLeftMostCol;
        Relation    idxRel;
        IndexInfo  *idxInfo;
 
        idxRel = index_open(idxoid, AccessShareLock);
        idxInfo = BuildIndexInfo(idxRel);
-       isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-       containsLeftMostCol =
-           RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
+       isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
        index_close(idxRel, AccessShareLock);
 
        /* Return the first eligible index found */
-       if (isUsableIdx && containsLeftMostCol)
+       if (isUsableIdx)
            return idxoid;
    }
 
@@ -832,17 +767,60 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 }
 
 /*
- * Returns true if the index is usable for replica identity full. For details,
- * see FindUsableIndexForReplicaIdentityFull.
+ * Returns true if the index is usable for replica identity full.
+ *
+ * The index must be btree, non-partial, and the leftmost field must be a
+ * column (not an expression) that references the remote relation column.
+ * These limitations help to keep the index scan similar to PK/RI index
+ * scans.
+ *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
+ *
+ * Note that the limitations of index scans for replica identity full only
+ * adheres to a subset of the limitations of PK/RI. For example, we support
+ * columns that are marked as [NULL] or we are not interested in the [NOT
+ * DEFERRABLE] aspect of constraints here. It works for us because we always
+ * compare the tuples for non-PK/RI index scans. See
+ * RelationFindReplTupleByIndex().
+ *
+ * XXX: There are no fundamental problems for supporting non-btree indexes.
+ * We mostly need to relax the limitations in RelationFindReplTupleByIndex().
+ * For partial indexes, the required changes are likely to be larger. If
+ * none of the tuples satisfy the expression for the index scan, we fall-back
+ * to sequential execution, which might not be a good idea in some cases.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
+IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 {
-   bool        is_btree = (indexInfo->ii_Am == BTREE_AM_OID);
-   bool        is_partial = (indexInfo->ii_Predicate != NIL);
-   bool        is_only_on_expression = IsIndexOnlyOnExpression(indexInfo);
+   AttrNumber  keycol;
+
+   /* The index must be a Btree index */
+   if (indexInfo->ii_Am != BTREE_AM_OID)
+       return false;
+
+   /* The index must not be a partial index */
+   if (indexInfo->ii_Predicate != NIL)
+       return false;
+
+   Assert(indexInfo->ii_NumIndexAttrs >= 1);
 
-   return is_btree && !is_partial && !is_only_on_expression;
+   /* The leftmost index field must not be an expression */
+   keycol = indexInfo->ii_IndexAttrNumbers[0];
+   if (!AttributeNumberIsValid(keycol))
+       return false;
+
+   /*
+    * And the leftmost index field must reference the remote relation column.
+    * This is because if it doesn't, the sequential scan is favorable over
+    * index scan in most cases.
+    */
+   if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+       attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
+       return false;
+
+   return true;
 }
 
 /*
index cb6659fc619b688629efba6b2577759f771962f2..832b1cf76422d9b979d7c74847dd89f8d8354f98 100644 (file)
 #include 
 #include 
 
+#include "access/genam.h"
 #include "access/table.h"
 #include "access/tableam.h"
 #include "access/twophase.h"
@@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
                                         ResultRelInfo *relinfo,
                                         TupleTableSlot *remoteslot,
                                         Oid localindexoid);
-static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
+static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
                                    LogicalRepRelation *remoterel,
                                    Oid localidxoid,
                                    TupleTableSlot *remoteslot,
@@ -2663,7 +2664,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
    EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
    ExecOpenIndices(relinfo, false);
 
-   found = FindReplTupleInLocalRel(estate, localrel,
+   found = FindReplTupleInLocalRel(edata, localrel,
                                    &relmapentry->remoterel,
                                    localindexoid,
                                    remoteslot, &localslot);
@@ -2816,7 +2817,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
    EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
    ExecOpenIndices(relinfo, false);
 
-   found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid,
+   found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid,
                                    remoteslot, &localslot);
 
    /* If found delete it. */
@@ -2855,12 +2856,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
  * Local tuple, if found, is returned in '*localslot'.
  */
 static bool
-FindReplTupleInLocalRel(EState *estate, Relation localrel,
+FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
                        LogicalRepRelation *remoterel,
                        Oid localidxoid,
                        TupleTableSlot *remoteslot,
                        TupleTableSlot **localslot)
 {
+   EState     *estate = edata->estate;
    bool        found;
 
    /*
@@ -2875,9 +2877,21 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
           (remoterel->replident == REPLICA_IDENTITY_FULL));
 
    if (OidIsValid(localidxoid))
+   {
+#ifdef USE_ASSERT_CHECKING
+       Relation    idxrel = index_open(localidxoid, AccessShareLock);
+
+       /* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
+       Assert(GetRelationIdentityOrPK(idxrel) == localidxoid ||
+              IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+                                                  edata->targetRel->attrmap));
+       index_close(idxrel, AccessShareLock);
+#endif
+
        found = RelationFindReplTupleByIndex(localrel, localidxoid,
                                             LockTupleExclusive,
                                             remoteslot, *localslot);
+   }
    else
        found = RelationFindReplTupleSeq(localrel, LockTupleExclusive,
                                         remoteslot, *localslot);
@@ -2995,7 +3009,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
                bool        found;
 
                /* Get the matching local tuple from the partition. */
-               found = FindReplTupleInLocalRel(estate, partrel,
+               found = FindReplTupleInLocalRel(edata, partrel,
                                                &part_entry->remoterel,
                                                part_entry->localindexoid,
                                                remoteslot_part, &localslot);
index 921b9974db78130d36713560079942354a1ff883..3f4d906d7416b138329765f11a8a24e0770632e5 100644 (file)
@@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
                                                        Relation partrel, AttrMap *map);
 extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
                                 LOCKMODE lockmode);
-extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo);
+extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap);
 extern Oid GetRelationIdentityOrPK(Relation rel);
 
 #endif                         /* LOGICALRELATION_H */