From 69d226a736a77ca83e8df2e617226552c431da13 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Sun, 26 Oct 2025 10:49:25 -0400 Subject: [PATCH v19 2/4] Track changed indexed columns in the executor during UPDATEs Refactor executor update logic to determine which indexed columns have actually changed during an UPDATE operation rather than leaving this up to HeapDetermineColumnsInfo in heap_update. This enables the comparison to happen without taking a lock on the page and opens the door to reuse in other code paths. Because heap_update now requires the caller to provide the modified indexed columns simple_heap_update has become a tad more complex. It is frequently called from CatalogTupleUpdate which either updates heap tuples via their form or using heap_modify_tuple. In both cases the caller does know the modified set of attributes, but sadly those attributes are lost before being provided to simple_heap_update. Due to that the "simple" path has to retain the HeapDetermineColumnsInfo logic of old (for now). In order for that to work it was necessary to split the (overly large) heap_update call itself up. This moves up into simple_heap_update and heap_tuple_update a bit of what existed in heap_update itself. Ideally this will be cleaned up once CatalogTupleUpdate paths are all recording modified attributes correctly, when that happens the "simple" path can be simplified again. ExecCheckIndexedAttrsForChanges replaces HeapDeterminesColumnsInfo and tts_attr_equal replaces heap_attr_equal changing the test for equality when calling into heap_tuple_update (but not simple_heap_update). In the past we used datumIsEqual(), essentially a binary comparison using memcmp(), now the comparison code in tts_attr_equal uses type-specific equality function when available and falls back to datumIsEqual() when not. This change in equality testing has some intended implications and opens the door for more HOT updates (foreshadowing). For instance, indexes with collation information allowing more HOT updates when the index is specified to be case insensitive. This change forced some logic changes in execReplication on the update paths is now it is required to have knowledge of the set of attributes that are both changed and referenced by indexes. Luckilly, the this is available within calls to slot_modify_data() where LogicalRepTupleData is processed and has a set of updated attributes. In this case rather than using ExecCheckIndexedAttrsForChanges we can preseve what slot_modify_data() identifies as the modified set and then intersect that with the set of indexes on the relation and get the correct set of modified indexed attributes required on heap_update(). --- src/backend/access/heap/heapam.c | 12 +- src/backend/access/heap/heapam_handler.c | 72 +++++-- src/backend/access/table/tableam.c | 5 +- src/backend/executor/execMain.c | 1 + src/backend/executor/execReplication.c | 7 + src/backend/executor/nodeModifyTable.c | 247 ++++++++++++++++++++++- src/backend/nodes/bitmapset.c | 4 + src/backend/replication/logical/worker.c | 72 ++++++- src/backend/utils/cache/relcache.c | 15 ++ src/include/access/tableam.h | 8 +- src/include/executor/executor.h | 5 + src/include/nodes/execnodes.h | 1 + src/include/utils/rel.h | 1 + src/include/utils/relcache.h | 1 + 14 files changed, 415 insertions(+), 36 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index aff47481345..1cdb72b3a7a 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3263,12 +3263,12 @@ simple_heap_delete(Relation relation, const ItemPointerData *tid) * generated by another transaction). */ TM_Result -heap_update(Relation relation, HeapTupleData *oldtup, - HeapTuple newtup, CommandId cid, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, LockTupleMode *lockmode, Buffer buffer, - Page page, BlockNumber block, ItemId lp, Bitmapset *hot_attrs, - Bitmapset *sum_attrs, Bitmapset *pk_attrs, Bitmapset *rid_attrs, - Bitmapset *mix_attrs, Buffer *vmbuffer, +heap_update(Relation relation, HeapTupleData *oldtup, HeapTuple newtup, + CommandId cid, Snapshot crosscheck, bool wait, + TM_FailureData *tmfd, LockTupleMode *lockmode, + Buffer buffer, Page page, BlockNumber block, ItemId lp, + Bitmapset *hot_attrs, Bitmapset *sum_attrs, Bitmapset *pk_attrs, + Bitmapset *rid_attrs, Bitmapset *mix_attrs, Buffer *vmbuffer, bool rep_id_key_required, TU_UpdateIndexes *update_indexes) { TM_Result result; diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 1cf9a18775d..ef08e1d3e10 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -315,9 +315,12 @@ heapam_tuple_delete(Relation relation, ItemPointer tid, CommandId cid, static TM_Result heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, - CommandId cid, Snapshot snapshot, Snapshot crosscheck, - bool wait, TM_FailureData *tmfd, - LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes) + CommandId cid, Snapshot snapshot, + Snapshot crosscheck, bool wait, + TM_FailureData *tmfd, + LockTupleMode *lockmode, + Bitmapset *mix_attrs, + TU_UpdateIndexes *update_indexes) { bool rep_id_key_required = false; bool shouldFree = true; @@ -332,7 +335,6 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, *sum_attrs, *pk_attrs, *rid_attrs, - *mix_attrs, *idx_attrs; TM_Result result; @@ -414,16 +416,61 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, oldtup.t_len = ItemIdGetLength(lp); oldtup.t_self = *otid; - mix_attrs = HeapDetermineColumnsInfo(relation, idx_attrs, rid_attrs, - &oldtup, tuple, &rep_id_key_required); - /* - * We'll need to WAL log the replica identity attributes if either they - * overlap with the modified indexed attributes or, as we've checked for - * just now in HeapDetermineColumnsInfo, they were unmodified external - * indexed attributes. + * We'll need to include the replica identity key when either the identity + * key attributes overlap with the modified index attributes or when the + * replica identity attributes are stored externally. This is required + * because for such attributes the flattened value won't be WAL logged as + * part of the new tuple so we must determine if we need to extract and + * include them as part of the old_key_tuple (see ExtractReplicaIdentity). */ - rep_id_key_required = rep_id_key_required || bms_overlap(mix_attrs, rid_attrs); + rep_id_key_required = bms_overlap(mix_attrs, rid_attrs); + if (!rep_id_key_required) + { + Bitmapset *attrs; + TupleDesc tupdesc = RelationGetDescr(relation); + int attidx = -1; + + /* + * We don't own idx_attrs so we'll copy it and remove the modified set + * to reduce the attributes we need to test in the while loop and + * avoid a two branches in the loop. + */ + attrs = bms_difference(idx_attrs, mix_attrs); + attrs = bms_int_members(attrs, rid_attrs); + + while ((attidx = bms_next_member(attrs, attidx)) >= 0) + { + /* + * attidx is zero-based, attrnum is the normal attribute number + */ + AttrNumber attrnum = attidx + FirstLowInvalidHeapAttributeNumber; + Datum value; + bool isnull; + + /* + * System attributes are not added into interesting_attrs in + * relcache + */ + Assert(attrnum > 0); + + value = heap_getattr(&oldtup, attrnum, tupdesc, &isnull); + + /* No need to check attributes that can't be stored externally */ + if (isnull || + TupleDescCompactAttr(tupdesc, attrnum - 1)->attlen != -1) + continue; + + /* Check if the old tuple's attribute is stored externally */ + if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value))) + { + rep_id_key_required = true; + break; + } + } + + bms_free(attrs); + } /* Update the tuple with table oid */ slot->tts_tableOid = RelationGetRelid(relation); @@ -437,7 +484,6 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, bms_free(sum_attrs); bms_free(pk_attrs); bms_free(rid_attrs); - bms_free(mix_attrs); bms_free(idx_attrs); ItemPointerCopy(&tuple->t_self, &slot->tts_tid); diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 5e41404937e..dadcf03ed24 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -336,6 +336,7 @@ void simple_table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot, Snapshot snapshot, + Bitmapset *modified_indexed_cols, TU_UpdateIndexes *update_indexes) { TM_Result result; @@ -346,7 +347,9 @@ simple_table_tuple_update(Relation rel, ItemPointer otid, GetCurrentCommandId(true), snapshot, InvalidSnapshot, true /* wait for commit */ , - &tmfd, &lockmode, update_indexes); + &tmfd, &lockmode, + modified_indexed_cols, + update_indexes); switch (result) { diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 27c9eec697b..6b7b6bc8019 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1282,6 +1282,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, /* The following fields are set later if needed */ resultRelInfo->ri_RowIdAttNo = 0; resultRelInfo->ri_extraUpdatedCols = NULL; + resultRelInfo->ri_ChangedIndexedCols = NULL; resultRelInfo->ri_projectNew = NULL; resultRelInfo->ri_newTupleSlot = NULL; resultRelInfo->ri_oldTupleSlot = NULL; diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index def32774c90..2709e2db0f2 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -32,6 +32,7 @@ #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/rel.h" +#include "utils/relcache.h" #include "utils/snapmgr.h" #include "utils/syscache.h" #include "utils/typcache.h" @@ -936,7 +937,13 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, if (rel->rd_rel->relispartition) ExecPartitionCheck(resultRelInfo, slot, estate, true); + /* + * We're not going to call ExecCheckIndexedAttrsForChanges here + * because we've already identified the changes earlier on thanks to + * slot_modify_data. + */ simple_table_tuple_update(rel, tid, slot, estate->es_snapshot, + resultRelInfo->ri_ChangedIndexedCols, &update_indexes); conflictindexes = resultRelInfo->ri_onConflictArbiterIndexes; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 4c5647ac38a..4d1cf50e369 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -17,6 +17,7 @@ * ExecModifyTable - retrieve the next tuple from the node * ExecEndModifyTable - shut down the ModifyTable node * ExecReScanModifyTable - rescan the ModifyTable node + * ExecCheckIndexedAttrsForChanges - find set of updated indexed columns * * NOTES * The ModifyTable node receives input from its outerPlan, which is @@ -54,11 +55,14 @@ #include "access/htup_details.h" #include "access/tableam.h" +#include "access/tupconvert.h" +#include "access/tupdesc.h" #include "access/xact.h" #include "commands/trigger.h" #include "executor/execPartition.h" #include "executor/executor.h" #include "executor/nodeModifyTable.h" +#include "executor/tuptable.h" #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" @@ -68,6 +72,8 @@ #include "storage/lmgr.h" #include "utils/builtins.h" #include "utils/datum.h" +#include "utils/float.h" +#include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/snapmgr.h" @@ -176,6 +182,219 @@ static TupleTableSlot *ExecMergeNotMatched(ModifyTableContext *context, bool canSetTag); +/* + * Compare two datums using the type's default equality operator. + * + * Returns true if the values are equal according to the type's equality + * operator, false otherwise. Falls back to binary comparison if no + * type-specific operator is available. + * + * This function uses the TypeCache infrastructure which caches operator + * lookups for efficiency. + */ +bool +tts_attr_equal(Oid typid, Oid collation, bool typbyval, int16 typlen, + Datum value1, Datum value2) +{ + TypeCacheEntry *typentry; + + LOCAL_FCINFO(fcinfo, 2); + Datum result; + + /* + * Fast path for common types to avoid even the type cache lookup. These + * types have simple equality semantics. + */ + switch (typid) + { + case INT2OID: + return DatumGetInt16(value1) == DatumGetInt16(value2); + case INT4OID: + return DatumGetInt32(value1) == DatumGetInt32(value2); + case INT8OID: + return DatumGetInt64(value1) == DatumGetInt64(value2); + case FLOAT4OID: + return !float4_cmp_internal(DatumGetFloat4(value1), DatumGetFloat4(value2)); + case FLOAT8OID: + return !float8_cmp_internal(DatumGetFloat8(value1), DatumGetFloat8(value2)); + case BOOLOID: + return DatumGetBool(value1) == DatumGetBool(value2); + case OIDOID: + case REGPROCOID: + case REGPROCEDUREOID: + case REGOPEROID: + case REGOPERATOROID: + case REGCLASSOID: + case REGTYPEOID: + case REGROLEOID: + case REGNAMESPACEOID: + case REGCONFIGOID: + case REGDICTIONARYOID: + return DatumGetObjectId(value1) == DatumGetObjectId(value2); + case CHAROID: + return DatumGetChar(value1) == DatumGetChar(value2); + default: + /* Continue to type cache lookup */ + break; + } + + /* + * Look up the type's equality operator using the type cache. Request both + * the operator OID and the function info for efficiency. + */ + typentry = lookup_type_cache(typid, + TYPECACHE_EQ_OPR | TYPECACHE_EQ_OPR_FINFO); + + /* + * If no equality operator is available, fall back to binary comparison. + * This handles types that don't have proper equality operators defined. + */ + if (!OidIsValid(typentry->eq_opr)) + return datumIsEqual(value1, value2, typbyval, typlen); + + /* + * Use the cached function info if available, otherwise look it up. The + * type cache keeps this around so subsequent calls are fast. + */ + if (typentry->eq_opr_finfo.fn_addr == NULL) + { + Oid eq_proc = get_opcode(typentry->eq_opr); + + if (!OidIsValid(eq_proc)) + /* Shouldn't happen, but fall back to binary comparison */ + return datumIsEqual(value1, value2, typbyval, typlen); + + fmgr_info_cxt(eq_proc, &typentry->eq_opr_finfo, + CacheMemoryContext); + } + + /* Set up function call */ + InitFunctionCallInfoData(*fcinfo, &typentry->eq_opr_finfo, 2, + collation, NULL, NULL); + + fcinfo->args[0].value = value1; + fcinfo->args[0].isnull = false; + fcinfo->args[1].value = value2; + fcinfo->args[1].isnull = false; + + /* Invoke the equality operator */ + result = FunctionCallInvoke(fcinfo); + + /* + * If the function returned NULL (shouldn't happen for equality ops), + * treat as not equal for safety. + */ + if (fcinfo->isnull) + return false; + + return DatumGetBool(result); +} + +/* + * Determine which updated attributes actually changed values between old and + * new tuples and are referenced by indexes on the relation. + * + * Returns a Bitmapset of attribute offsets (0-based, adjusted by + * FirstLowInvalidHeapAttributeNumber) or NULL if no attributes changed. + */ +Bitmapset * +ExecCheckIndexedAttrsForChanges(ResultRelInfo *relinfo, + TupleTableSlot *tts_old, + TupleTableSlot *tts_new) +{ + Relation relation = relinfo->ri_RelationDesc; + TupleDesc tupdesc = RelationGetDescr(relation); + Bitmapset *indexed_attrs; + Bitmapset *modified = NULL; + int attidx; + + /* If no indexes, we're done */ + if (relinfo->ri_NumIndices == 0) + return NULL; + + /* + * Get the set of index key attributes. This includes summarizing, + * expression indexes and attributes mentioned in the predicate of a + * partition but not those in INCLUDING. + */ + indexed_attrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_INDEXED); + Assert(!bms_is_empty(indexed_attrs)); + + /* + * NOTE: It is important to scan all indexed attributes in the tuples + * because ExecGetAllUpdatedCols won't include columns that may have been + * modified via heap_modify_tuple_by_col which is the case in + * tsvector_update_trigger. + */ + attidx = -1; + while ((attidx = bms_next_member(indexed_attrs, attidx)) >= 0) + { + /* attidx is zero-based, attrnum is the normal attribute number */ + AttrNumber attrnum = attidx + FirstLowInvalidHeapAttributeNumber; + Form_pg_attribute attr; + bool oldnull, + newnull; + Datum oldval, + newval; + + /* + * If it's a whole-tuple reference, record as modified. It's not + * really worth supporting this case, since it could only succeed + * after a no-op update, which is hardly a case worth optimizing for. + */ + if (attrnum == 0) + { + modified = bms_add_member(modified, attidx); + continue; + } + + /* + * Likewise, include in the modified set any system attribute other + * than tableOID; we cannot expect these to be consistent in a HOT + * chain, or even to be set correctly yet in the new tuple. + */ + if (attrnum < 0) + { + if (attrnum != TableOidAttributeNumber) + modified = bms_add_member(modified, attidx); + continue; + } + + /* Extract values from both slots */ + oldval = slot_getattr(tts_old, attrnum, &oldnull); + newval = slot_getattr(tts_new, attrnum, &newnull); + + /* If one value is NULL and the other is not, they are not equal */ + if (oldnull != newnull) + { + modified = bms_add_member(modified, attidx); + continue; + } + + /* If both are NULL, consider them equal */ + if (oldnull) + continue; + + /* Get attribute metadata */ + Assert(attrnum > 0 && attrnum <= tupdesc->natts); + attr = TupleDescAttr(tupdesc, attrnum - 1); + + /* Compare using type-specific equality operator */ + if (!tts_attr_equal(attr->atttypid, + attr->attcollation, + attr->attbyval, + attr->attlen, + oldval, + newval)) + modified = bms_add_member(modified, attidx); + } + + bms_free(indexed_attrs); + + return modified; +} + /* * Verify that the tuples to be produced by INSERT match the * target relation's rowtype @@ -2168,8 +2387,8 @@ ExecUpdatePrepareSlot(ResultRelInfo *resultRelInfo, */ static TM_Result ExecUpdateAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo, - ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot *slot, - bool canSetTag, UpdateContext *updateCxt) + ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot *oldSlot, + TupleTableSlot *slot, bool canSetTag, UpdateContext *updateCxt) { EState *estate = context->estate; Relation resultRelationDesc = resultRelInfo->ri_RelationDesc; @@ -2291,6 +2510,16 @@ lreplace: if (resultRelationDesc->rd_att->constr) ExecConstraints(resultRelInfo, slot, estate); + /* + * Identify which, if any, indexed attributes were modified here so that + * we might reuse it in a few places. + */ + bms_free(resultRelInfo->ri_ChangedIndexedCols); + resultRelInfo->ri_ChangedIndexedCols = NULL; + + resultRelInfo->ri_ChangedIndexedCols = + ExecCheckIndexedAttrsForChanges(resultRelInfo, oldSlot, slot); + /* * replace the heap tuple * @@ -2306,6 +2535,7 @@ lreplace: estate->es_crosscheck_snapshot, true /* wait for commit */ , &context->tmfd, &updateCxt->lockmode, + resultRelInfo->ri_ChangedIndexedCols, &updateCxt->updateIndexes); return result; @@ -2524,8 +2754,9 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo, */ redo_act: lockedtid = *tupleid; - result = ExecUpdateAct(context, resultRelInfo, tupleid, oldtuple, slot, - canSetTag, &updateCxt); + + result = ExecUpdateAct(context, resultRelInfo, tupleid, oldtuple, oldSlot, + slot, canSetTag, &updateCxt); /* * If ExecUpdateAct reports that a cross-partition update was done, @@ -3222,8 +3453,8 @@ lmerge_matched: Assert(oldtuple == NULL); result = ExecUpdateAct(context, resultRelInfo, tupleid, - NULL, newslot, canSetTag, - &updateCxt); + NULL, resultRelInfo->ri_oldTupleSlot, + newslot, canSetTag, &updateCxt); /* * As in ExecUpdate(), if ExecUpdateAct() reports that a @@ -3248,6 +3479,7 @@ lmerge_matched: tupleid, NULL, newslot); mtstate->mt_merge_updated += 1; } + break; case CMD_DELETE: @@ -4333,7 +4565,7 @@ ExecModifyTable(PlanState *pstate) * For UPDATE/DELETE/MERGE, fetch the row identity info for the tuple * to be updated/deleted/merged. For a heap relation, that's a TID; * otherwise we may have a wholerow junk attr that carries the old - * tuple in toto. Keep this in step with the part of + * tuple in total. Keep this in step with the part of * ExecInitModifyTable that sets up ri_RowIdAttNo. */ if (operation == CMD_UPDATE || operation == CMD_DELETE || @@ -4509,6 +4741,7 @@ ExecModifyTable(PlanState *pstate) /* Now apply the update. */ slot = ExecUpdate(&context, resultRelInfo, tupleid, oldtuple, oldSlot, slot, node->canSetTag); + if (tuplock) UnlockTuple(resultRelInfo->ri_RelationDesc, tupleid, InplaceUpdateTupleLock); diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c index b4ecf0b0390..9014990267a 100644 --- a/src/backend/nodes/bitmapset.c +++ b/src/backend/nodes/bitmapset.c @@ -238,6 +238,10 @@ bms_make_singleton(int x) void bms_free(Bitmapset *a) { +#if USE_ASSERT_CHECKING + Assert(bms_is_valid_set(a)); +#endif + if (a) pfree(a); } diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 93970c6af29..b363eaa49cc 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -243,6 +243,8 @@ */ #include "postgres.h" +#include "access/sysattr.h" +#include "nodes/bitmapset.h" #include #include @@ -275,7 +277,6 @@ #include "replication/logicalrelation.h" #include "replication/logicalworker.h" #include "replication/origin.h" -#include "replication/slot.h" #include "replication/walreceiver.h" #include "replication/worker_internal.h" #include "rewrite/rewriteHandler.h" @@ -291,6 +292,7 @@ #include "utils/memutils.h" #include "utils/pg_lsn.h" #include "utils/rel.h" +#include "utils/relcache.h" #include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/syscache.h" @@ -1110,15 +1112,18 @@ slot_store_data(TupleTableSlot *slot, LogicalRepRelMapEntry *rel, * "slot" is filled with a copy of the tuple in "srcslot", replacing * columns provided in "tupleData" and leaving others as-is. * + * Returns a bitmap of the modified columns. + * * Caution: unreplaced pass-by-ref columns in "slot" will point into the * storage for "srcslot". This is OK for current usage, but someday we may * need to materialize "slot" at the end to make it independent of "srcslot". */ -static void +static Bitmapset * slot_modify_data(TupleTableSlot *slot, TupleTableSlot *srcslot, LogicalRepRelMapEntry *rel, LogicalRepTupleData *tupleData) { + Bitmapset *modified = NULL; int natts = slot->tts_tupleDescriptor->natts; int i; @@ -1195,6 +1200,28 @@ slot_modify_data(TupleTableSlot *slot, TupleTableSlot *srcslot, slot->tts_isnull[i] = true; } + /* + * Determine if the replicated value changed the local value by + * comparing slots. This is a subset of + * ExecCheckIndexedAttrsForChanges. + */ + if (srcslot->tts_isnull[i] != slot->tts_isnull[i]) + { + /* One is NULL, the other is not so the value changed */ + modified = bms_add_member(modified, i + 1 - FirstLowInvalidHeapAttributeNumber); + } + else if (!srcslot->tts_isnull[i]) + { + /* Both are not NULL, compare their values */ + if (!tts_attr_equal(att->atttypid, + att->attcollation, + att->attbyval, + att->attlen, + srcslot->tts_values[i], + slot->tts_values[i])) + modified = bms_add_member(modified, i + 1 - FirstLowInvalidHeapAttributeNumber); + } + /* Reset attnum for error callback */ apply_error_callback_arg.remote_attnum = -1; } @@ -1202,6 +1229,8 @@ slot_modify_data(TupleTableSlot *slot, TupleTableSlot *srcslot, /* And finally, declare that "slot" contains a valid virtual tuple */ ExecStoreVirtualTuple(slot); + + return modified; } /* @@ -2918,6 +2947,7 @@ apply_handle_update_internal(ApplyExecutionData *edata, ConflictTupleInfo conflicttuple = {0}; bool found; MemoryContext oldctx; + Bitmapset *indexed = NULL; EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); ExecOpenIndices(relinfo, false); @@ -2934,6 +2964,8 @@ apply_handle_update_internal(ApplyExecutionData *edata, */ if (found) { + Bitmapset *modified = NULL; + /* * Report the conflict if the tuple was modified by a different * origin. @@ -2957,15 +2989,29 @@ apply_handle_update_internal(ApplyExecutionData *edata, /* Process and store remote tuple in the slot */ oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); - slot_modify_data(remoteslot, localslot, relmapentry, newtup); + modified = slot_modify_data(remoteslot, localslot, relmapentry, newtup); MemoryContextSwitchTo(oldctx); + /* + * Normally we'd call ExecCheckIndexedAttrForChanges but here we have + * the record of changed columns in the replication state, so let's + * use that instead. + */ + indexed = RelationGetIndexAttrBitmap(relinfo->ri_RelationDesc, + INDEX_ATTR_BITMAP_INDEXED); + + bms_free(relinfo->ri_ChangedIndexedCols); + relinfo->ri_ChangedIndexedCols = bms_int_members(modified, indexed); + bms_free(indexed); + EvalPlanQualSetSlot(&epqstate, remoteslot); InitConflictIndexes(relinfo); - /* Do the actual update. */ + /* First check privileges */ TargetPrivilegesCheck(relinfo->ri_RelationDesc, ACL_UPDATE); + + /* Then do the actual update. */ ExecSimpleRelationUpdate(relinfo, estate, &epqstate, localslot, remoteslot); } @@ -3455,6 +3501,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, bool found; EPQState epqstate; ConflictTupleInfo conflicttuple = {0}; + Bitmapset *modified = NULL; + Bitmapset *indexed; /* Get the matching local tuple from the partition. */ found = FindReplTupleInLocalRel(edata, partrel, @@ -3523,8 +3571,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, * remoteslot_part. */ oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); - slot_modify_data(remoteslot_part, localslot, part_entry, - newtup); + modified = slot_modify_data(remoteslot_part, localslot, part_entry, + newtup); MemoryContextSwitchTo(oldctx); EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); @@ -3549,6 +3597,18 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, EvalPlanQualSetSlot(&epqstate, remoteslot_part); TargetPrivilegesCheck(partrelinfo->ri_RelationDesc, ACL_UPDATE); + + /* + * Normally we'd call ExecCheckIndexedAttrForChanges but + * here we have the record of changed columns in the + * replication state, so let's use that instead. + */ + indexed = RelationGetIndexAttrBitmap(partrelinfo->ri_RelationDesc, + INDEX_ATTR_BITMAP_INDEXED); + bms_free(partrelinfo->ri_ChangedIndexedCols); + partrelinfo->ri_ChangedIndexedCols = bms_int_members(modified, indexed); + bms_free(indexed); + ExecSimpleRelationUpdate(partrelinfo, estate, &epqstate, localslot, remoteslot_part); } diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 915d0bc9084..32825596be1 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -2482,6 +2482,7 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) bms_free(relation->rd_idattr); bms_free(relation->rd_hotblockingattr); bms_free(relation->rd_summarizedattr); + bms_free(relation->rd_indexedattr); if (relation->rd_pubdesc) pfree(relation->rd_pubdesc); if (relation->rd_options) @@ -5283,6 +5284,7 @@ RelationGetIndexPredicate(Relation relation) * index (empty if FULL) * INDEX_ATTR_BITMAP_HOT_BLOCKING Columns that block updates from being HOT * INDEX_ATTR_BITMAP_SUMMARIZED Columns included in summarizing indexes + * INDEX_ATTR_BITMAP_INDEXED Columns referenced by indexes * * Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that * we can include system attributes (e.g., OID) in the bitmap representation. @@ -5307,6 +5309,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind) Bitmapset *idindexattrs; /* columns in the replica identity */ Bitmapset *hotblockingattrs; /* columns with HOT blocking indexes */ Bitmapset *summarizedattrs; /* columns with summarizing indexes */ + Bitmapset *indexedattrs; /* columns referenced by indexes */ List *indexoidlist; List *newindexoidlist; Oid relpkindex; @@ -5329,6 +5332,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind) return bms_copy(relation->rd_hotblockingattr); case INDEX_ATTR_BITMAP_SUMMARIZED: return bms_copy(relation->rd_summarizedattr); + case INDEX_ATTR_BITMAP_INDEXED: + return bms_copy(relation->rd_indexedattr); default: elog(ERROR, "unknown attrKind %u", attrKind); } @@ -5373,6 +5378,7 @@ restart: idindexattrs = NULL; hotblockingattrs = NULL; summarizedattrs = NULL; + indexedattrs = NULL; foreach(l, indexoidlist) { Oid indexOid = lfirst_oid(l); @@ -5505,10 +5511,14 @@ restart: bms_free(idindexattrs); bms_free(hotblockingattrs); bms_free(summarizedattrs); + bms_free(indexedattrs); goto restart; } + /* Combine all index attributes */ + indexedattrs = bms_union(hotblockingattrs, summarizedattrs); + /* Don't leak the old values of these bitmaps, if any */ relation->rd_attrsvalid = false; bms_free(relation->rd_keyattr); @@ -5521,6 +5531,8 @@ restart: relation->rd_hotblockingattr = NULL; bms_free(relation->rd_summarizedattr); relation->rd_summarizedattr = NULL; + bms_free(relation->rd_indexedattr); + relation->rd_indexedattr = NULL; /* * Now save copies of the bitmaps in the relcache entry. We intentionally @@ -5535,6 +5547,7 @@ restart: relation->rd_idattr = bms_copy(idindexattrs); relation->rd_hotblockingattr = bms_copy(hotblockingattrs); relation->rd_summarizedattr = bms_copy(summarizedattrs); + relation->rd_indexedattr = bms_copy(indexedattrs); relation->rd_attrsvalid = true; MemoryContextSwitchTo(oldcxt); @@ -5551,6 +5564,8 @@ restart: return hotblockingattrs; case INDEX_ATTR_BITMAP_SUMMARIZED: return summarizedattrs; + case INDEX_ATTR_BITMAP_INDEXED: + return indexedattrs; default: elog(ERROR, "unknown attrKind %u", attrKind); return NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index e16bf025692..8a5931a3118 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -549,6 +549,7 @@ typedef struct TableAmRoutine bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, + Bitmapset *updated_cols, TU_UpdateIndexes *update_indexes); /* see table_tuple_lock() for reference about parameters */ @@ -1502,12 +1503,12 @@ static inline TM_Result table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot, CommandId cid, Snapshot snapshot, Snapshot crosscheck, bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, - TU_UpdateIndexes *update_indexes) + Bitmapset *updated_cols, TU_UpdateIndexes *update_indexes) { return rel->rd_tableam->tuple_update(rel, otid, slot, cid, snapshot, crosscheck, - wait, tmfd, - lockmode, update_indexes); + wait, tmfd, lockmode, + updated_cols, update_indexes); } /* @@ -2010,6 +2011,7 @@ extern void simple_table_tuple_delete(Relation rel, ItemPointer tid, Snapshot snapshot); extern void simple_table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot, Snapshot snapshot, + Bitmapset *modified_indexe_attrs, TU_UpdateIndexes *update_indexes); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index fa2b657fb2f..993dc0e6ced 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -800,5 +800,10 @@ extern ResultRelInfo *ExecLookupResultRelByOid(ModifyTableState *node, Oid resultoid, bool missing_ok, bool update_cache); +extern Bitmapset *ExecCheckIndexedAttrsForChanges(ResultRelInfo *resultRelInfo, + TupleTableSlot *tts_old, + TupleTableSlot *tts_new); +extern bool tts_attr_equal(Oid typid, Oid collation, bool typbyval, int16 typlen, + Datum value1, Datum value2); #endif /* EXECUTOR_H */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 18ae8f0d4bb..8b08e0045ba 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -498,6 +498,7 @@ typedef struct ResultRelInfo Bitmapset *ri_extraUpdatedCols; /* true if the above has been computed */ bool ri_extraUpdatedCols_valid; + Bitmapset *ri_ChangedIndexedCols; /* Projection to generate new tuple in an INSERT/UPDATE */ ProjectionInfo *ri_projectNew; diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 80286076a11..b23a7306e69 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -164,6 +164,7 @@ typedef struct RelationData Bitmapset *rd_idattr; /* included in replica identity index */ Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */ Bitmapset *rd_summarizedattr; /* cols indexed by summarizing indexes */ + Bitmapset *rd_indexedattr; /* all cols referenced by indexes */ PublicationDesc *rd_pubdesc; /* publication descriptor, or NULL */ diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 3561c6bef0b..d3fbb8b093a 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -71,6 +71,7 @@ typedef enum IndexAttrBitmapKind INDEX_ATTR_BITMAP_IDENTITY_KEY, INDEX_ATTR_BITMAP_HOT_BLOCKING, INDEX_ATTR_BITMAP_SUMMARIZED, + INDEX_ATTR_BITMAP_INDEXED, } IndexAttrBitmapKind; extern Bitmapset *RelationGetIndexAttrBitmap(Relation relation, -- 2.49.0