From c957288f978aec59e8bd4ebc6b35039520f818b2 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 4 Dec 2024 12:06:24 +0100 Subject: [PATCH v45.3 3/5] Make the conditions in IsIndexUsableForReplicaIdentityFull() more explicit IsIndexUsableForReplicaIdentityFull() described a number of conditions that a suitable index has to fulfill. But not all of these were actually checked in the code. Instead, it appeared to rely on get_equal_strategy_number() to filter out any indexes that are not btree or hash. As we look to generalize index AM capabilities, this would possibly break if we added additional support in get_equal_strategy_number(). Instead, write out code to check for the required capabilities explicitly. This shouldn't change any behaviors at the moment. Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com --- src/backend/replication/logical/relation.c | 52 +++++++++++----------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index c3799a6185e..dd8a3809096 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -17,9 +17,7 @@ #include "postgres.h" -#ifdef USE_ASSERT_CHECKING #include "access/amapi.h" -#endif #include "access/genam.h" #include "access/table.h" #include "catalog/namespace.h" @@ -798,9 +796,10 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) /* * Returns true if the index is usable for replica identity full. * - * The index must be btree or hash, 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. + * The index must have an equal strategy for each key column, be 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 @@ -813,19 +812,6 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) * compare the tuples for non-PK/RI index scans. See * RelationFindReplTupleByIndex(). * - * The reasons why only Btree and Hash indexes can be considered as usable are: - * - * 1) Other index access methods don't have a fixed strategy for equality - * operation. Refer get_equal_strategy_number(). - * - * 2) For indexes other than PK and REPLICA IDENTITY, we need to match the - * local and remote tuples. The equality routine tuples_equal() cannot accept - * a datatype (e.g. point or box) that does not have a default operator class - * for Btree or Hash. - * - * XXX: Note that BRIN and GIN indexes do not implement "amgettuple" which - * will be used later to fetch the tuples. See RelationFindReplTupleByIndex(). - * * XXX: To support 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. @@ -853,6 +839,21 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap) return false; } + /* + * For indexes other than PK and REPLICA IDENTITY, we need to match the + * local and remote tuples. The equality routine tuples_equal() cannot + * accept a data type where the type cache cannot provide an equality + * operator. + */ + for (int i = 0; i < idxrel->rd_att->natts; i++) + { + TypeCacheEntry *typentry; + + typentry = lookup_type_cache(TupleDescAttr(idxrel->rd_att, i)->atttypid, TYPECACHE_EQ_OPR_FINFO); + if (!OidIsValid(typentry->eq_opr_finfo.fn_oid)) + return false; + } + /* The leftmost index field must not be an expression */ keycol = idxrel->rd_index->indkey.values[0]; if (!AttributeNumberIsValid(keycol)) @@ -867,15 +868,12 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap) attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0) return false; -#ifdef USE_ASSERT_CHECKING - { - IndexAmRoutine *amroutine; - - /* The given index access method must implement amgettuple. */ - amroutine = GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false); - Assert(amroutine->amgettuple != NULL); - } -#endif + /* + * The given index access method must implement "amgettuple", which will + * be used later to fetch the tuples. See RelationFindReplTupleByIndex(). + */ + if (GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false)->amgettuple == NULL) + return false; return true; } -- 2.42.0