Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 13544e7

Browse files
committed
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. Reviewed-by: Paul Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
1 parent a2a475b commit 13544e7

File tree

1 file changed

+25
-27
lines changed

1 file changed

+25
-27
lines changed

src/backend/replication/logical/relation.c

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717

1818
#include "postgres.h"
1919

20-
#ifdef USE_ASSERT_CHECKING
2120
#include "access/amapi.h"
22-
#endif
2321
#include "access/genam.h"
2422
#include "access/table.h"
2523
#include "catalog/namespace.h"
@@ -798,9 +796,10 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
798796
/*
799797
* Returns true if the index is usable for replica identity full.
800798
*
801-
* The index must be btree or hash, non-partial, and the leftmost field must be
802-
* a column (not an expression) that references the remote relation column. These
803-
* limitations help to keep the index scan similar to PK/RI index scans.
799+
* The index must have an equal strategy for each key column, be non-partial,
800+
* and the leftmost field must be a column (not an expression) that references
801+
* the remote relation column. These limitations help to keep the index scan
802+
* similar to PK/RI index scans.
804803
*
805804
* attrmap is a map of local attributes to remote ones. We can consult this
806805
* map to check whether the local index attribute has a corresponding remote
@@ -813,19 +812,6 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
813812
* compare the tuples for non-PK/RI index scans. See
814813
* RelationFindReplTupleByIndex().
815814
*
816-
* The reasons why only Btree and Hash indexes can be considered as usable are:
817-
*
818-
* 1) Other index access methods don't have a fixed strategy for equality
819-
* operation. Refer get_equal_strategy_number().
820-
*
821-
* 2) For indexes other than PK and REPLICA IDENTITY, we need to match the
822-
* local and remote tuples. The equality routine tuples_equal() cannot accept
823-
* a datatype (e.g. point or box) that does not have a default operator class
824-
* for Btree or Hash.
825-
*
826-
* XXX: Note that BRIN and GIN indexes do not implement "amgettuple" which
827-
* will be used later to fetch the tuples. See RelationFindReplTupleByIndex().
828-
*
829815
* XXX: To support partial indexes, the required changes are likely to be larger.
830816
* If none of the tuples satisfy the expression for the index scan, we fall-back
831817
* to sequential execution, which might not be a good idea in some cases.
@@ -853,6 +839,21 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
853839
return false;
854840
}
855841

842+
/*
843+
* For indexes other than PK and REPLICA IDENTITY, we need to match the
844+
* local and remote tuples. The equality routine tuples_equal() cannot
845+
* accept a data type where the type cache cannot provide an equality
846+
* operator.
847+
*/
848+
for (int i = 0; i < idxrel->rd_att->natts; i++)
849+
{
850+
TypeCacheEntry *typentry;
851+
852+
typentry = lookup_type_cache(TupleDescAttr(idxrel->rd_att, i)->atttypid, TYPECACHE_EQ_OPR_FINFO);
853+
if (!OidIsValid(typentry->eq_opr_finfo.fn_oid))
854+
return false;
855+
}
856+
856857
/* The leftmost index field must not be an expression */
857858
keycol = idxrel->rd_index->indkey.values[0];
858859
if (!AttributeNumberIsValid(keycol))
@@ -867,15 +868,12 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
867868
attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
868869
return false;
869870

870-
#ifdef USE_ASSERT_CHECKING
871-
{
872-
IndexAmRoutine *amroutine;
873-
874-
/* The given index access method must implement amgettuple. */
875-
amroutine = GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false);
876-
Assert(amroutine->amgettuple != NULL);
877-
}
878-
#endif
871+
/*
872+
* The given index access method must implement "amgettuple", which will
873+
* be used later to fetch the tuples. See RelationFindReplTupleByIndex().
874+
*/
875+
if (GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false)->amgettuple == NULL)
876+
return false;
879877

880878
return true;
881879
}

0 commit comments

Comments
 (0)