From b6e397e7d9d81ab0d13f31cc236a0e04b6c2d5bd Mon Sep 17 00:00:00 2001 From: Antonin Houska Date: Wed, 1 Jul 2026 15:18:10 +0200 Subject: [PATCH 2/2] Provide the executor with information on updated columns. When replaying data changes, REPACK calls ExecInsertIndexTuples() for each INSERT and UPDATE. In the latter case, the function needs to determine the value of the 'indexUnchanged' hint for the index AM. (The hint is always false for INSERT.) Therefore, REPACK is supposed to specify which columns are changed by the UPDATE. So far, REPACK missed to specify the set of updated columns, so the 'indexUnchanged' hint could incorrectly evaluate to true. Although it should not affect correctness, it can make the index AM use optimizations that are not appropriate. Ideally, we should compute the set of updated columns for each individual UPDATE, however the comparison of the old and new tuple might add too much overhead. This patch initializes the set as if all columns were updated. Thus 'indexUnchanged' always evaluates to false. This way the index AM never uses the related optimizations. It might result in worse structure of the index, however it seems better to not use the optimizations than to misuse them. --- src/backend/commands/repack.c | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index c9b0c047477..f37bb9a21af 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -62,6 +62,7 @@ #include "libpq/pqmq.h" #include "miscadmin.h" #include "optimizer/optimizer.h" +#include "parser/parse_relation.h" #include "pgstat.h" #include "replication/logicalrelation.h" #include "storage/bufmgr.h" @@ -3005,13 +3006,50 @@ static void initialize_change_context(ChangeContext *chgcxt, Relation relation, Oid ident_index_id) { + Bitmapset *updatedCols = NULL; + RangeTblEntry *rte; + List *perminfos = NIL; + RTEPermissionInfo *perminfo; + chgcxt->cc_rel = relation; /* Only initialize fields needed by ExecInsertIndexTuples(). */ chgcxt->cc_estate = CreateExecutorState(); + /* + * Initialize updatedCols. + * + * The point is that ExecInsertIndexTuples() should not pass the + * indexUnchanged hint to the index AM unless there's a reason to do + * so. For simplicity, we consider all columns updated. + * + * XXX Should we spend more effort to compare the old and new tuple when + * replaying UPDATE, or at least exclude unchanged TOAST values, like we + * do in logicalrep_write_tuple()? + */ + for (int i = 0; i < RelationGetDescr(relation)->natts; i++) + updatedCols = bms_add_member(updatedCols, + i + 1 - FirstLowInvalidHeapAttributeNumber); + + /* + * In this case, RTE only needs to have ->perminfoindex initialized, but + * there's no reason to not set the fields whose values we have at hand. + */ + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = RelationGetRelid(relation); + rte->relkind = RelationGetForm(relation)->relkind; + /* Create the RTEPermissionInfo instance (and set ->perminfoindex). */ + addRTEPermissionInfo(&perminfos, rte); + /* Make updatedCols available to the executor functions. */ + perminfo = getRTEPermissionInfo(perminfos, rte); + perminfo->updatedCols = updatedCols; + + ExecInitRangeTable(chgcxt->cc_estate, list_make1(rte), perminfos, + bms_make_singleton(1)); + chgcxt->cc_rri = makeNode(ResultRelInfo); - InitResultRelInfo(chgcxt->cc_rri, relation, 0, NULL, 0); + InitResultRelInfo(chgcxt->cc_rri, relation, 1, NULL, 0); ExecOpenIndices(chgcxt->cc_rri, false); /* -- 2.52.0