From 9f8df08f25b5545add41736a3db08c27f690f218 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Sun, 18 Oct 2026 16:43:01 -0400 Subject: [PATCH v41 09/10] Keep HOT for exclusion constraints, and check them after the update An exclusion constraint is checked when its index gets an entry, against the values, and values that give the same index entries need not have the same conflicts (hashint8(1) = hashint8(2^32)). So the previous commits never let an exclusion constraint's index keep its entries when one of its values changed: such an update was never HOT. DocumentDB builds every unique index as an exclusion constraint on the whole document, so a collection with one got no HOT updates at all. Let it keep them, and check the constraint anyway. When amcomparedatums says an exclusion constraint's index would get the same entries although a value changed, ExecUpdateModifiedIdxAttrs() notes the index in the ResultRelInfo, and if the table AM then makes the update HOT, ExecCheckHotExclusion() runs check_exclusion_or_unique_constraint() for the new tuple with the new values, choosing the wait mode and deferred handling as ExecInsertIndexTuples() does (both now share that choice). The old entries lead to the new tuple through the HOT chain, so the constraint's index scan finds it as "self", and finds every conflicting row it would have found had the new entry been inserted. A concurrent inserter of a conflicting value finds the in-progress new tuple through those entries, and waits for it; our check finds the inserter's entry, and waits for it. core already checks a HOT child this way in unique_key_recheck(). UPDATE, MERGE, logical replication apply and REPACK all check after a HOT update. For a deferred constraint the index is returned for the AFTER trigger queue, as for an insertion. unique_key_recheck() had to change too. It found the row it rechecks by TID with table_fetch_tid(), which takes the TID as the root of a HOT chain: a heap-only tuple there ends the search, so a recheck queued for the new version found it "dead" and passed. And it checked the values of the version it was queued for, which a later HOT update may since have changed, as the comment there assumes cannot happen. For an exclusion constraint it now follows the row's updates to the version visible to SnapshotSelf and checks that version's values; a unique index keeps the existing behavior. Tests: hash_index covers immediate and deferred constraints: HOT when the values don't conflict, an error when they do, a recheck at commit that sees the row's later update, the other row's update, and the row's deletion, and SET CONSTRAINTS IMMEDIATE. hot-exclusion (isolation) covers a concurrent insertion before and after the HOT update, committed and rolled back. 032_subscribe_use_index covers apply. Disabling the check after a HOT update, not noting the index, or reverting unique_key_recheck each make a test fail. --- doc/src/sgml/indexam.sgml | 7 +- doc/src/sgml/storage.sgml | 5 +- src/backend/access/heap/README.HOT | 16 +- src/backend/commands/constraint.c | 64 ++++++-- src/backend/commands/repack.c | 5 + src/backend/executor/execIndexing.c | 147 ++++++++++++++---- src/backend/executor/execReplication.c | 17 ++ src/backend/executor/nodeModifyTable.c | 76 +++++++-- src/backend/utils/cache/relcache.c | 10 +- src/include/executor/executor.h | 4 + src/include/nodes/execnodes.h | 11 ++ src/test/isolation/expected/hot-exclusion.out | 93 +++++++++++ src/test/isolation/isolation_schedule | 1 + src/test/isolation/specs/hot-exclusion.spec | 50 ++++++ src/test/regress/expected/hash_index.out | 86 +++++++++- src/test/regress/sql/hash_index.sql | 54 ++++++- .../subscription/t/032_subscribe_use_index.pl | 51 ++++++ 17 files changed, 626 insertions(+), 71 deletions(-) create mode 100644 src/test/isolation/expected/hot-exclusion.out create mode 100644 src/test/isolation/specs/hot-exclusion.spec diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index 18b6698e38f..eced8543202 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -955,8 +955,11 @@ amcomparedatums (Relation indexRelation, is always correct. It must not modify the index, and it may be called for an index that is not yet valid. Columns of unique indexes that can be referenced by a foreign key, and of the replica identity index, are not - compared this way, and the function is not called for the index of an - exclusion constraint, whose check needs the values themselves. If amcomparedatums is + compared this way. For the index of an exclusion constraint, the + function's answer only means the entries are the same: the executor then + checks the constraint itself, against the new values, whenever the + update makes no new index entries. If + amcomparedatums is NULL, a change to a column that the index stores always needs new index entries. diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 15b5dae1752..d8b5b8bc13f 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -1125,8 +1125,9 @@ data. Empty in ordinary tables. value gives the same index entries as the old one; among the built-in methods, that depends on the method and on the change (see ). This does not apply to the columns of - unique indexes that a foreign key can reference, of the replica - identity index, or of exclusion constraints. + unique indexes that a foreign key can reference or of the replica + identity index. It does apply to exclusion constraints, which are then + checked against the new values although no index entry is made. diff --git a/src/backend/access/heap/README.HOT b/src/backend/access/heap/README.HOT index 5c2dfc27f58..44f0da658eb 100644 --- a/src/backend/access/heap/README.HOT +++ b/src/backend/access/heap/README.HOT @@ -44,9 +44,19 @@ byte for byte, and not whether the tuple satisfies their predicates. The same holds for a column that indexes also store directly, if every index that does so has an access method that can tell, from the old and new values, that it would store the same entries (amcomparedatums), and -the column is in no candidate key, replica identity or exclusion -constraint. The executor works that out before calling the table AM, -and passes on only the indexed columns whose change matters. An index +the column is in no candidate key or replica identity. The executor +works that out before calling the table AM, and passes on only the +indexed columns whose change matters. + +An exclusion constraint is checked when its index gets an entry, against +the values, and values that give the same entries need not have the same +conflicts. So when an exclusion constraint's index keeps its entries +although a value changed, and the update turns out to be HOT, the +executor checks the constraint after the update itself, for the new +tuple, as inserting its entry would have. The old entries lead to the +new tuple through the HOT chain. A deferred constraint's recheck at +commit follows the row's updates to its live version and checks that +version's values. An index that is not yet ready for inserts, or whose build failed, counts as changed whenever a column it references does; see "CREATE INDEX CONCURRENTLY" below. diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c index d1d383ddb7a..66839c28116 100644 --- a/src/backend/commands/constraint.c +++ b/src/backend/commands/constraint.c @@ -22,6 +22,26 @@ #include "utils/snapmgr.h" +/* + * Follow the updates of the row whose version is at *tid to its version + * visible to SnapshotSelf, if any: set *tid to it, store it in slot, and + * return true. Returns false if there is none. + */ +static bool +row_live_version(Relation rel, ItemPointer tid, TupleTableSlot *slot) +{ + TableScanDesc scan = table_beginscan_tid(rel, SnapshotSelf); + ItemPointerData latest = *tid; + bool live; + + table_tuple_get_latest_tid(scan, &latest); + table_endscan(scan); + live = table_tuple_fetch_row_version(rel, &latest, SnapshotSelf, slot); + if (live) + *tid = latest; + return live; +} + /* * unique_key_recheck - trigger function to do a deferred uniqueness check. * @@ -47,6 +67,7 @@ unique_key_recheck(PG_FUNCTION_ARGS) EState *estate; ExprContext *econtext; TupleTableSlot *slot; + TupleTableSlot *liveslot = NULL; Datum values[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; @@ -102,25 +123,46 @@ unique_key_recheck(PG_FUNCTION_ARGS) * if it doesn't find the index entry for the row. If the row's dead then * it's possible the index entry has also been marked dead, and even * removed. + * + * An exclusion constraint's values need not be the same along the chain, + * though: a HOT update keeps the index's entries when its access method + * says the new values give the same ones, and then checks the constraint + * itself, queueing this recheck for the new version if it is deferred + * (see ExecCheckHotExclusion()). That version is a heap-only tuple, + * which table_fetch_tid() cannot start from, and a later update may have + * changed the values again. So for an exclusion constraint, follow the + * row's updates to the live version and check its values. + * + * Open the index first, acquiring a RowExclusiveLock, just as if we were + * going to update it. (This protects against possible changes of the + * index schema, not against concurrent updates.) */ + indexRel = index_open(trigdata->tg_trigger->tgconstrindid, + RowExclusiveLock); + indexInfo = BuildIndexInfo(indexRel); + tmptid = checktid; - if (!table_fetch_tid(trigdata->tg_relation, &tmptid, SnapshotSelf, NULL)) + if (indexInfo->ii_ExclusionOps != NULL) + { + liveslot = table_slot_create(trigdata->tg_relation, NULL); + if (!row_live_version(trigdata->tg_relation, &tmptid, liveslot)) + { + ExecDropSingleTupleTableSlot(liveslot); + index_close(indexRel, RowExclusiveLock); + return PointerGetDatum(NULL); + } + slot = liveslot; + } + else if (!table_fetch_tid(trigdata->tg_relation, &tmptid, SnapshotSelf, + NULL)) { /* * All rows referenced by the index entry are dead, so skip the check */ + index_close(indexRel, RowExclusiveLock); return PointerGetDatum(NULL); } - /* - * Open the index, acquiring a RowExclusiveLock, just as if we were going - * to update it. (This protects against possible changes of the index - * schema, not against concurrent updates.) - */ - indexRel = index_open(trigdata->tg_trigger->tgconstrindid, - RowExclusiveLock); - indexInfo = BuildIndexInfo(indexRel); - /* * Typically the index won't have expressions, but if it does we need an * EState to evaluate them. We need it for exclusion constraints too, @@ -185,6 +227,8 @@ unique_key_recheck(PG_FUNCTION_ARGS) */ if (estate != NULL) FreeExecutorState(estate); + if (liveslot != NULL) + ExecDropSingleTupleTableSlot(liveslot); index_close(indexRel, RowExclusiveLock); diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 4294c860e12..359499ec1d7 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2952,6 +2952,11 @@ apply_concurrent_update(Relation rel, TupleTableSlot *spilled_tuple, spilled_tuple, NIL, NULL); } + if (update_indexes != TU_All) + (void) ExecCheckHotExclusion(chgcxt->cc_rri, chgcxt->cc_estate, + EIIT_IS_UPDATE, spilled_tuple, NIL, NULL); + bms_free(chgcxt->cc_rri->ri_HotExclusionCheck); + chgcxt->cc_rri->ri_HotExclusionCheck = NULL; pgstat_progress_incr_param(PROGRESS_REPACK_HEAP_TUPLES_UPDATED, 1); } diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index d0a714a42f2..532a73cfdbe 100644 --- a/src/backend/executor/execIndexing.c +++ b/src/backend/executor/execIndexing.c @@ -140,6 +140,12 @@ static bool check_exclusion_or_unique_constraint(Relation heap, Relation index, static bool index_recheck_constraint(Relation index, const Oid *constr_procs, const Datum *existing_values, const bool *existing_isnull, const Datum *new_values); +static bool check_exclusion_for_insert(Relation heapRelation, + Relation indexRelation, + IndexInfo *indexInfo, + ItemPointer tupleid, + const Datum *values, const bool *isnull, + EState *estate, bool applyNoDupErr); static bool index_unchanged_by_update(ResultRelInfo *resultRelInfo, EState *estate, IndexInfo *indexInfo, Relation indexRelation); @@ -471,33 +477,10 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, * need to preserve the prior state of satisfiesConstraint. */ if (indexInfo->ii_ExclusionOps != NULL) - { - bool violationOK; - CEOUC_WAIT_MODE waitMode; - - if (applyNoDupErr) - { - violationOK = true; - waitMode = CEOUC_LIVELOCK_PREVENTING_WAIT; - } - else if (!indexRelation->rd_index->indimmediate) - { - violationOK = true; - waitMode = CEOUC_NOWAIT; - } - else - { - violationOK = false; - waitMode = CEOUC_WAIT; - } - satisfiesConstraint = - check_exclusion_or_unique_constraint(heapRelation, - indexRelation, indexInfo, - tupleid, values, isnull, - estate, false, - waitMode, violationOK, NULL); - } + check_exclusion_for_insert(heapRelation, indexRelation, + indexInfo, tupleid, values, isnull, + estate, applyNoDupErr); if ((checkUnique == UNIQUE_CHECK_PARTIAL || indexInfo->ii_ExclusionOps != NULL) && @@ -518,6 +501,118 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, return result; } +/* + * Check the exclusion constraint of indexRelation for a tuple whose entry has + * just been inserted, or whose entries are those an earlier version of the + * row had (a HOT update; see ExecCheckHotExclusion()). Returns false if the + * constraint may be violated and the caller is to recheck it later. + * + * If the constraint is deferred, check now anyway, but don't throw error on + * violation or wait for a conclusive outcome from a concurrent insertion; + * the caller queues a recheck event. Similarly, noDupErr callers + * (speculative inserters) will recheck later, and wait for a conclusive + * outcome then. + */ +static bool +check_exclusion_for_insert(Relation heapRelation, Relation indexRelation, + IndexInfo *indexInfo, ItemPointer tupleid, + const Datum *values, const bool *isnull, + EState *estate, bool applyNoDupErr) +{ + bool violationOK; + CEOUC_WAIT_MODE waitMode; + + if (applyNoDupErr) + { + violationOK = true; + waitMode = CEOUC_LIVELOCK_PREVENTING_WAIT; + } + else if (!indexRelation->rd_index->indimmediate) + { + violationOK = true; + waitMode = CEOUC_NOWAIT; + } + else + { + violationOK = false; + waitMode = CEOUC_WAIT; + } + + return check_exclusion_or_unique_constraint(heapRelation, indexRelation, + indexInfo, tupleid, + values, isnull, estate, false, + waitMode, violationOK, NULL); +} + +/* ---------------------------------------------------------------- + * ExecCheckHotExclusion + * + * After an UPDATE that inserted no index entries (a HOT update), + * check the exclusion constraints that ExecUpdateModifiedIdxAttrs() + * noted in ri_HotExclusionCheck: those whose index keeps the old row's + * entries although a value it is built from changed. The entries + * still lead to the new row, through the HOT chain, so the check is + * the one ExecInsertIndexTuples() would have made after inserting + * the new row's entry, made without inserting it. + * + * Like ExecInsertIndexTuples(), this returns the OIDs of indexes of + * deferred constraints that had potential conflicts, to be rechecked + * at commit, and honors EIIT_NO_DUPE_ERROR and arbiterIndexes. + * specConflict is set as there. + * ---------------------------------------------------------------- + */ +List * +ExecCheckHotExclusion(ResultRelInfo *resultRelInfo, EState *estate, + uint32 flags, TupleTableSlot *slot, + List *arbiterIndexes, bool *specConflict) +{ + ItemPointer tupleid = &slot->tts_tid; + Relation heapRelation = resultRelInfo->ri_RelationDesc; + ExprContext *econtext; + List *result = NIL; + int i = -1; + + if (bms_is_empty(resultRelInfo->ri_HotExclusionCheck)) + return NIL; + Assert(ItemPointerIsValid(tupleid)); + + econtext = GetPerTupleExprContext(estate); + econtext->ecxt_scantuple = slot; + + while ((i = bms_next_member(resultRelInfo->ri_HotExclusionCheck, i)) >= 0) + { + Relation indexRelation = resultRelInfo->ri_IndexRelationDescs[i]; + IndexInfo *indexInfo = resultRelInfo->ri_IndexRelationInfo[i]; + Datum values[INDEX_MAX_KEYS]; + bool isnull[INDEX_MAX_KEYS]; + bool applyNoDupErr; + + Assert(indexInfo->ii_ExclusionOps != NULL); + + /* + * ExecUpdateModifiedIdxAttrs() only notes an index the row is in, so + * a partial index's predicate holds. + */ + FormIndexDatum(indexInfo, slot, estate, values, isnull); + + applyNoDupErr = (flags & EIIT_NO_DUPE_ERROR) && + (arbiterIndexes == NIL || + list_member_oid(arbiterIndexes, + indexRelation->rd_index->indexrelid)); + + if (!check_exclusion_for_insert(heapRelation, indexRelation, indexInfo, + tupleid, values, isnull, estate, + applyNoDupErr)) + { + result = lappend_oid(result, RelationGetRelid(indexRelation)); + if (indexRelation->rd_index->indimmediate && specConflict) + *specConflict = true; + } + } + + return result; +} + /* ---------------------------------------------------------------- * ExecCheckIndexConstraints * diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index fda7db4bb0a..6b7d0d5e62b 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -994,6 +994,23 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, &conflict); } + /* + * If no non-summarizing index got an entry, check the exclusion + * constraints whose entries the row kept although their values + * changed. + */ + if (update_indexes != TU_All) + recheckIndexes = + list_concat(recheckIndexes, + ExecCheckHotExclusion(resultRelInfo, estate, + conflictindexes != NIL ? + EIIT_IS_UPDATE | EIIT_NO_DUPE_ERROR : + EIIT_IS_UPDATE, + slot, conflictindexes, + &conflict)); + bms_free(resultRelInfo->ri_HotExclusionCheck); + resultRelInfo->ri_HotExclusionCheck = NULL; + /* * Refer to the comments above the call to CheckAndReportConflict() in * ExecSimpleRelationInsert to understand why this check is done at diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index f78c1bb7787..ffe3a6d8b65 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -205,19 +205,22 @@ static TupleTableSlot *ExecMergeNotMatched(ModifyTableContext *context, * differ byte for byte may still give the index the same entries; if its * access method can tell (amcomparedatums), ask it. * - * Not for an exclusion constraint's index, though: the constraint is checked - * when an entry is inserted, with its operators, against the values, and two - * values that give the same entries need not have the same conflicts. (A - * value that is the same byte for byte has, so the check is not needed.) + * *valchanged is set if some datum differs byte for byte although the + * entries are the same. That matters for an exclusion constraint's index: + * the constraint is checked on the values, and values that give the same + * entries need not have the same conflicts, so the caller must check the + * constraint itself if the update keeps the old entries. */ static bool IndexDatumsDiffer(Relation heapRel, Relation indexRel, IndexInfo *indexInfo, const Datum *old_values, const bool *old_isnull, - const Datum *new_values, const bool *new_isnull) + const Datum *new_values, const bool *new_isnull, + bool *valchanged) { ListCell *expr = list_head(indexInfo->ii_Expressions); - amcomparedatums_function cmp = indexRel->rd_index->indisexclusion ? NULL : - indexRel->rd_indam->amcomparedatums; + amcomparedatums_function cmp = indexRel->rd_indam->amcomparedatums; + + *valchanged = false; for (int i = 0; i < indexInfo->ii_NumIndexAttrs; i++) { @@ -251,6 +254,7 @@ IndexDatumsDiffer(Relation heapRel, Relation indexRel, IndexInfo *indexInfo, !cmp(indexRel, indexInfo, i + 1, typid, old_values[i], new_values[i])) return true; + *valchanged = true; } return false; } @@ -267,7 +271,7 @@ IndexDatumsDiffer(Relation heapRel, Relation indexRel, IndexInfo *indexInfo, static bool IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, IndexInfo *indexInfo, TupleTableSlot *old_tts, - TupleTableSlot *new_tts, EState *estate) + TupleTableSlot *new_tts, EState *estate, bool *valchanged) { ExprContext *econtext = GetPerTupleExprContext(estate); TupleTableSlot *save_scantuple = econtext->ecxt_scantuple; @@ -278,6 +282,7 @@ IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, bool new_isnull[INDEX_MAX_KEYS]; bool result; + *valchanged = false; if (indexInfo->ii_Predicate != NIL) { bool old_member; @@ -313,7 +318,7 @@ IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, result = IndexDatumsDiffer(resultRelInfo->ri_RelationDesc, indexRel, indexInfo, old_values, old_isnull, - new_values, new_isnull); + new_values, new_isnull, valchanged); MemoryContextSwitchTo(oldcxt); return result; } @@ -343,8 +348,7 @@ IndexExprAttrs(ResultRelInfo *resultRelInfo, int i, EState *estate) pull_varattnos((Node *) ii->ii_Predicate, 1, &attrs); if (ii->ii_Predicate != NIL || (resultRelInfo->ri_IndexRelationDescs[j] != NULL && - resultRelInfo->ri_IndexRelationDescs[j]->rd_indam->amcomparedatums != NULL && - !resultRelInfo->ri_IndexRelationDescs[j]->rd_index->indisexclusion)) + resultRelInfo->ri_IndexRelationDescs[j]->rd_indam->amcomparedatums != NULL)) { for (int k = 0; k < ii->ii_NumIndexAttrs; k++) { @@ -371,11 +375,13 @@ IndexExprAttrs(ResultRelInfo *resultRelInfo, int i, EState *estate) static bool IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, TupleTableSlot *old_tts, TupleTableSlot *new_tts, - EState *estate) + EState *estate, bool *valchanged) { IndexInfo *indexInfo = resultRelInfo->ri_IndexRelationInfo[i]; bool proven = false; + *valchanged = false; + if (resultRelInfo->ri_IndexUntouchedAttrs != NULL && bms_is_subset(mine, resultRelInfo->ri_IndexUntouchedAttrs[i])) { @@ -395,13 +401,19 @@ IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, } } + /* + * A proof says the index reads nothing the update changes, so its datums + * are the same byte for byte and *valchanged stays false. + */ #ifdef USE_ASSERT_CHECKING if (proven) { + bool changed; + if (IndexEntryChanges(resultRelInfo, resultRelInfo->ri_IndexRelationDescs[i], indexInfo, old_tts, new_tts, - estate)) + estate, &changed) || changed) elog(ERROR, "index \"%s\" proven unaffected by UPDATE, but its entry changed", RelationGetRelationName(resultRelInfo->ri_IndexRelationDescs[i])); return true; @@ -414,7 +426,7 @@ IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, return !IndexEntryChanges(resultRelInfo, resultRelInfo->ri_IndexRelationDescs[i], indexInfo, old_tts, new_tts, - estate); + estate, valchanged); } /* @@ -453,6 +465,13 @@ IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, * is unaffected when, column by column, its datums are equal or its access * method's amcomparedatums says they give the same entries. * + * That includes the index of an exclusion constraint. But the constraint is + * checked on the values, when an entry is inserted, and two values that give + * the same entries need not have the same conflicts. So when such an index + * keeps its entries although a value changed, it is noted in + * ri_HotExclusionCheck, and the caller must call ExecCheckHotExclusion() + * after the update if the update inserts no index entries. + * * An index that is not ready for inserts or not valid counts as affected by * any change to an attribute it references, without evaluating anything. * Such an index is being built, or its build failed, so the old row may be @@ -478,6 +497,10 @@ ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, Bitmapset *candidates; int attidx = -1; + /* what follows describes this row */ + bms_free(resultRelInfo->ri_HotExclusionCheck); + resultRelInfo->ri_HotExclusionCheck = NULL; + if (resultRelInfo->ri_NumIndices == 0) return NULL; @@ -561,6 +584,7 @@ ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, */ { Bitmapset *keep = NULL; + bool valchanged; for (int i = 0; i < resultRelInfo->ri_NumIndices; i++) { @@ -582,7 +606,7 @@ ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, if (!indexInfo->ii_ReadyForInserts || !resultRelInfo->ri_IndexRelationDescs[i]->rd_index->indisvalid || !IndexUnaffected(resultRelInfo, i, mine, old_tts, new_tts, - estate)) + estate, &valchanged)) { keep = bms_add_members(keep, mine); if (bms_is_subset(candidates, keep)) @@ -591,6 +615,14 @@ ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, break; } } + else if (valchanged && indexInfo->ii_ExclusionOps != NULL) + { + MemoryContext oldcxt = MemoryContextSwitchTo(estate->es_query_cxt); + + resultRelInfo->ri_HotExclusionCheck = + bms_add_member(resultRelInfo->ri_HotExclusionCheck, i); + MemoryContextSwitchTo(oldcxt); + } bms_free(mine); } @@ -2946,6 +2978,20 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, NULL); } + /* + * If no non-summarizing index got an entry, check the exclusion + * constraints whose entries the row kept although their values changed. + */ + if (updateCxt->updateIndexes != TU_All && + resultRelInfo->ri_HotExclusionCheck != NULL) + recheckIndexes = list_concat(recheckIndexes, + ExecCheckHotExclusion(resultRelInfo, + context->estate, + EIIT_IS_UPDATE, + slot, NIL, NULL)); + bms_free(resultRelInfo->ri_HotExclusionCheck); + resultRelInfo->ri_HotExclusionCheck = NULL; + /* AFTER ROW UPDATE Triggers */ ExecARUpdateTriggers(context->estate, resultRelInfo, NULL, NULL, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index f264dd1c858..a92927e7b49 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5531,8 +5531,7 @@ restart: else plainattrs = bms_add_member(plainattrs, attrnum - FirstLowInvalidHeapAttributeNumber); - if (indexDesc->rd_indam->amcomparedatums != NULL && - !indexDesc->rd_index->indisexclusion) + if (indexDesc->rd_indam->amcomparedatums != NULL) cmpattrs = bms_add_member(cmpattrs, attrnum - FirstLowInvalidHeapAttributeNumber); else @@ -5623,10 +5622,9 @@ restart: * out what summarizing indexes reference. They also leave out the * columns of candidate keys and the replica identity, whose changes are * decided by value as before: the table AM takes its tuple lock strength - * and what it logs for logical decoding from them. Indexes of exclusion - * constraints count as unable to compare: the constraint is checked on - * the values when an entry is inserted, and values that give the same - * entries need not have the same conflicts. + * and what it logs for logical decoding from them. The columns of + * exclusion constraints are included: if an update keeps such an index's + * entries, the executor checks the constraint itself. */ cmpattrs = bms_del_members(cmpattrs, nocmpattrs); cmpattrs = bms_del_members(cmpattrs, summarizedattrs); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index c82def87f2a..a8217593b6c 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -760,6 +760,10 @@ extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, EState *estate, uint32 flags, TupleTableSlot *slot, List *arbiterIndexes, bool *specConflict); +extern List *ExecCheckHotExclusion(ResultRelInfo *resultRelInfo, + EState *estate, uint32 flags, + TupleTableSlot *slot, List *arbiterIndexes, + bool *specConflict); extern bool ExecCheckIndexConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, ItemPointer conflictTid, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7c914ee001d..38d6caa44cf 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -523,6 +523,17 @@ typedef struct ResultRelInfo Bitmapset **ri_IndexUntouchedAttrs; int ri_UpdateCount; + /* + * For UPDATE, the indexes of exclusion constraints, as positions in the + * index arrays, that ExecUpdateModifiedIdxAttrs() let the row being + * updated keep its entries in although their values changed (their access + * method said the entries were the same). If the update is then HOT, no + * index entry is inserted, so the constraint must be checked separately; + * see ExecCheckHotExclusion(). Describes the row whose update was + * decided last. + */ + Bitmapset *ri_HotExclusionCheck; + /* * For UPDATE/DELETE/MERGE result relations, the attribute number of the * row identity junk attribute in the source plan's output tuples diff --git a/src/test/isolation/expected/hot-exclusion.out b/src/test/isolation/expected/hot-exclusion.out new file mode 100644 index 00000000000..1da28972b49 --- /dev/null +++ b/src/test/isolation/expected/hot-exclusion.out @@ -0,0 +1,93 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin s1_upd s1_hot s2_ins s1_commit s2_check +step s1_begin: BEGIN; +step s1_upd: UPDATE hot_excl SET k = 4294967296 WHERE id = 1; +step s1_hot: SELECT pg_stat_get_xact_tuples_hot_updated('hot_excl'::regclass) AS hot; +hot +--- + 1 +(1 row) + +step s2_ins: INSERT INTO hot_excl VALUES (2, 4294967296); +step s1_commit: COMMIT; +step s2_ins: <... completed> +ERROR: conflicting key value violates exclusion constraint "hot_excl_k_excl" +step s2_check: + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT id, k FROM hot_excl WHERE k = 4294967296 ORDER BY id; + RESET enable_seqscan; + RESET enable_bitmapscan; + +id| k +--+---------- + 1|4294967296 +(1 row) + + +starting permutation: s1_begin s1_upd s2_ins s1_abort s2_check +step s1_begin: BEGIN; +step s1_upd: UPDATE hot_excl SET k = 4294967296 WHERE id = 1; +step s2_ins: INSERT INTO hot_excl VALUES (2, 4294967296); +step s1_abort: ROLLBACK; +step s2_ins: <... completed> +step s2_check: + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT id, k FROM hot_excl WHERE k = 4294967296 ORDER BY id; + RESET enable_seqscan; + RESET enable_bitmapscan; + +id| k +--+---------- + 2|4294967296 +(1 row) + + +starting permutation: s2_begin s2_ins s1_upd s2_commit s2_check +step s2_begin: BEGIN; +step s2_ins: INSERT INTO hot_excl VALUES (2, 4294967296); +step s1_upd: UPDATE hot_excl SET k = 4294967296 WHERE id = 1; +step s2_commit: COMMIT; +step s1_upd: <... completed> +ERROR: conflicting key value violates exclusion constraint "hot_excl_k_excl" +step s2_check: + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT id, k FROM hot_excl WHERE k = 4294967296 ORDER BY id; + RESET enable_seqscan; + RESET enable_bitmapscan; + +id| k +--+---------- + 2|4294967296 +(1 row) + + +starting permutation: s2_begin s2_ins s1_begin s1_upd s2_abort s1_hot s1_commit s2_check +step s2_begin: BEGIN; +step s2_ins: INSERT INTO hot_excl VALUES (2, 4294967296); +step s1_begin: BEGIN; +step s1_upd: UPDATE hot_excl SET k = 4294967296 WHERE id = 1; +step s2_abort: ROLLBACK; +step s1_upd: <... completed> +step s1_hot: SELECT pg_stat_get_xact_tuples_hot_updated('hot_excl'::regclass) AS hot; +hot +--- + 1 +(1 row) + +step s1_commit: COMMIT; +step s2_check: + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT id, k FROM hot_excl WHERE k = 4294967296 ORDER BY id; + RESET enable_seqscan; + RESET enable_bitmapscan; + +id| k +--+---------- + 1|4294967296 +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index b76a884d646..817b35ad153 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -97,6 +97,7 @@ test: skip-locked-4 test: drop-index-concurrently-1 test: multiple-cic test: cic-expression-hot +test: hot-exclusion test: alter-domain-validate test: alter-table-1 test: alter-table-2 diff --git a/src/test/isolation/specs/hot-exclusion.spec b/src/test/isolation/specs/hot-exclusion.spec new file mode 100644 index 00000000000..04e3dabb43b --- /dev/null +++ b/src/test/isolation/specs/hot-exclusion.spec @@ -0,0 +1,50 @@ +# HOT updates and exclusion constraints, concurrently. +# +# hashint8() folds the high half of a value into the low half, so 1 and +# 2^32 have the same hash code: a hash index gives them the same entry, and +# changing a row's k from 1 to 2^32 can be a HOT update. The exclusion +# constraint must still be checked against the new value, and a concurrent +# insertion of the same value must still see the update, and the update +# the insertion, whichever comes first. + +setup +{ + CREATE TABLE hot_excl (id int, k int8, EXCLUDE USING hash (k WITH =)) + WITH (fillfactor = 10); + INSERT INTO hot_excl VALUES (1, 1); +} + +teardown +{ + DROP TABLE hot_excl; +} + +session s1 +step s1_begin { BEGIN; } +step s1_upd { UPDATE hot_excl SET k = 4294967296 WHERE id = 1; } +step s1_hot { SELECT pg_stat_get_xact_tuples_hot_updated('hot_excl'::regclass) AS hot; } +step s1_commit { COMMIT; } +step s1_abort { ROLLBACK; } + +session s2 +step s2_begin { BEGIN; } +step s2_ins { INSERT INTO hot_excl VALUES (2, 4294967296); } +step s2_commit { COMMIT; } +step s2_abort { ROLLBACK; } +step s2_check +{ + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT id, k FROM hot_excl WHERE k = 4294967296 ORDER BY id; + RESET enable_seqscan; + RESET enable_bitmapscan; +} + +# the HOT update comes first: the insertion waits for it, then fails +permutation s1_begin s1_upd s1_hot s2_ins s1_commit s2_check +# ... or succeeds if the update rolls back +permutation s1_begin s1_upd s2_ins s1_abort s2_check +# the insertion comes first: the HOT update waits for it, then fails +permutation s2_begin s2_ins s1_upd s2_commit s2_check +# ... or succeeds, and is HOT, if the insertion rolls back +permutation s2_begin s2_ins s1_begin s1_upd s2_abort s1_hot s1_commit s2_check diff --git a/src/test/regress/expected/hash_index.out b/src/test/regress/expected/hash_index.out index 9a1cf7e9f73..dd906e63d40 100644 --- a/src/test/regress/expected/hash_index.out +++ b/src/test/regress/expected/hash_index.out @@ -456,8 +456,9 @@ RESET enable_indexscan; RESET enable_seqscan; DROP TABLE hash_hot; -- An exclusion constraint is checked on the values, so a change to a value --- with the same hash code must still make a new entry and be checked. --- hashint8 folds the high half into the low, so 1 and 2^32 collide. +-- with the same hash code must still be checked, even when the index keeps +-- its entry and the update is HOT. hashint8 folds the high half into the +-- low, so 1 and 2^32 collide. SELECT hashint8(1) = hashint8(4294967296) AS same_hash; same_hash ----------- @@ -479,12 +480,12 @@ SELECT pg_stat_force_next_flush(); BEGIN; DELETE FROM hash_hot WHERE id = 2; --- without a conflict, the update is checked and is not HOT +-- without a conflict, the update is checked, and is HOT UPDATE hash_hot SET k = 4294967296 WHERE id = 1; SELECT * FROM hash_hot_count(); updated | hot ---------+----- - 1 | 0 + 1 | 1 (1 row) COMMIT; @@ -494,5 +495,82 @@ SELECT * FROM hash_hot; 1 | 4294967296 (1 row) +-- with a conflict, the HOT update fails +INSERT INTO hash_hot VALUES (3, 1); +UPDATE hash_hot SET k = 4294967296 WHERE id = 3; +ERROR: conflicting key value violates exclusion constraint "hash_hot_k_excl" +DETAIL: Key (k)=(4294967296) conflicts with existing key (k)=(4294967296). +-- the index still finds both rows under their values +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT * FROM hash_hot WHERE k = 4294967296; + id | k +----+------------ + 1 | 4294967296 +(1 row) + +SELECT * FROM hash_hot WHERE k = 1; + id | k +----+--- + 3 | 1 +(1 row) + +RESET enable_bitmapscan; +RESET enable_seqscan; +DROP TABLE hash_hot; +-- A deferred exclusion constraint is checked at commit, on the values the +-- row has then. +CREATE TABLE hash_hot (id int, k int8, + CONSTRAINT hash_hot_k EXCLUDE USING hash (k WITH =) + DEFERRABLE INITIALLY DEFERRED) + WITH (fillfactor = 10); +INSERT INTO hash_hot VALUES (1, 1), (2, 4294967296); +SELECT pg_stat_force_next_flush(); + pg_stat_force_next_flush +-------------------------- + +(1 row) + +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +SELECT * FROM hash_hot_count(); + updated | hot +---------+----- + 1 | 1 +(1 row) + +COMMIT; +ERROR: conflicting key value violates exclusion constraint "hash_hot_k" +DETAIL: Key (k)=(4294967296) conflicts with existing key (k)=(4294967296). +-- updated again before commit, back to no conflict +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +UPDATE hash_hot SET k = 1 WHERE id = 1; +COMMIT; +-- the other row moves away before commit +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +UPDATE hash_hot SET k = 3 WHERE id = 2; +COMMIT; +-- the row is deleted before commit +BEGIN; +UPDATE hash_hot SET k = 3 WHERE id = 1; +DELETE FROM hash_hot WHERE id = 1; +COMMIT; +-- checked when the constraint is made immediate +BEGIN; +UPDATE hash_hot SET k = 3 WHERE id = 2; +INSERT INTO hash_hot VALUES (4, 4294967296); +UPDATE hash_hot SET k = 3 WHERE id = 4; +SET CONSTRAINTS hash_hot_k IMMEDIATE; +ERROR: conflicting key value violates exclusion constraint "hash_hot_k" +DETAIL: Key (k)=(3) conflicts with existing key (k)=(3). +ROLLBACK; +SELECT * FROM hash_hot ORDER BY id; + id | k +----+--- + 2 | 3 +(1 row) + DROP TABLE hash_hot; DROP FUNCTION hash_hot_count; diff --git a/src/test/regress/sql/hash_index.sql b/src/test/regress/sql/hash_index.sql index f7edc33312f..6b3e490b0be 100644 --- a/src/test/regress/sql/hash_index.sql +++ b/src/test/regress/sql/hash_index.sql @@ -389,8 +389,9 @@ RESET enable_seqscan; DROP TABLE hash_hot; -- An exclusion constraint is checked on the values, so a change to a value --- with the same hash code must still make a new entry and be checked. --- hashint8 folds the high half into the low, so 1 and 2^32 collide. +-- with the same hash code must still be checked, even when the index keeps +-- its entry and the update is HOT. hashint8 folds the high half into the +-- low, so 1 and 2^32 collide. SELECT hashint8(1) = hashint8(4294967296) AS same_hash; CREATE TABLE hash_hot (id int, k int8, EXCLUDE USING hash (k WITH =)) WITH (fillfactor = 10); @@ -400,10 +401,57 @@ UPDATE hash_hot SET k = 4294967296 WHERE id = 1; SELECT pg_stat_force_next_flush(); BEGIN; DELETE FROM hash_hot WHERE id = 2; --- without a conflict, the update is checked and is not HOT +-- without a conflict, the update is checked, and is HOT UPDATE hash_hot SET k = 4294967296 WHERE id = 1; SELECT * FROM hash_hot_count(); COMMIT; SELECT * FROM hash_hot; +-- with a conflict, the HOT update fails +INSERT INTO hash_hot VALUES (3, 1); +UPDATE hash_hot SET k = 4294967296 WHERE id = 3; +-- the index still finds both rows under their values +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT * FROM hash_hot WHERE k = 4294967296; +SELECT * FROM hash_hot WHERE k = 1; +RESET enable_bitmapscan; +RESET enable_seqscan; +DROP TABLE hash_hot; + +-- A deferred exclusion constraint is checked at commit, on the values the +-- row has then. +CREATE TABLE hash_hot (id int, k int8, + CONSTRAINT hash_hot_k EXCLUDE USING hash (k WITH =) + DEFERRABLE INITIALLY DEFERRED) + WITH (fillfactor = 10); +INSERT INTO hash_hot VALUES (1, 1), (2, 4294967296); +SELECT pg_stat_force_next_flush(); +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +SELECT * FROM hash_hot_count(); +COMMIT; +-- updated again before commit, back to no conflict +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +UPDATE hash_hot SET k = 1 WHERE id = 1; +COMMIT; +-- the other row moves away before commit +BEGIN; +UPDATE hash_hot SET k = 4294967296 WHERE id = 1; +UPDATE hash_hot SET k = 3 WHERE id = 2; +COMMIT; +-- the row is deleted before commit +BEGIN; +UPDATE hash_hot SET k = 3 WHERE id = 1; +DELETE FROM hash_hot WHERE id = 1; +COMMIT; +-- checked when the constraint is made immediate +BEGIN; +UPDATE hash_hot SET k = 3 WHERE id = 2; +INSERT INTO hash_hot VALUES (4, 4294967296); +UPDATE hash_hot SET k = 3 WHERE id = 4; +SET CONSTRAINTS hash_hot_k IMMEDIATE; +ROLLBACK; +SELECT * FROM hash_hot ORDER BY id; DROP TABLE hash_hot; DROP FUNCTION hash_hot_count; diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl index bd80465f727..ad154128b3d 100644 --- a/src/test/subscription/t/032_subscribe_use_index.pl +++ b/src/test/subscription/t/032_subscribe_use_index.pl @@ -733,6 +733,57 @@ SKIP: # demoted from replica identity # ============================================================================= +# ============================================================================= +# Testcase start: an applied UPDATE can be HOT on the subscriber although it +# changes a column of an exclusion constraint, whose index keeps its entry +# (hashint8() gives 1 and 2^32 the same hash code), and the constraint is +# still checked there. + +$node_publisher->safe_psql('postgres', + "CREATE TABLE test_hot_excl (id int PRIMARY KEY, k int8)"); +$node_subscriber->safe_psql('postgres', + "CREATE TABLE test_hot_excl (id int PRIMARY KEY, k int8, EXCLUDE USING hash (k WITH =)) WITH (fillfactor = 10)" +); +$node_publisher->safe_psql('postgres', + "INSERT INTO test_hot_excl VALUES (1, 1), (2, 2)"); +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION tap_pub_hot_excl FOR TABLE test_hot_excl"); +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION tap_sub_hot_excl CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_hot_excl" +); +$node_subscriber->wait_for_subscription_sync($node_publisher, $appname); + +$node_publisher->safe_psql('postgres', + "UPDATE test_hot_excl SET k = 4294967296 WHERE id = 1"); +$node_publisher->wait_for_catchup($appname); +# the apply worker reports its counts asynchronously +ok( $node_subscriber->poll_query_until( + 'postgres', + "SELECT n_tup_upd = 1 AND n_tup_hot_upd = 1 FROM pg_stat_user_tables WHERE relname = 'test_hot_excl'" + ), + 'applied UPDATE of an exclusion constraint column is HOT'); +$result = $node_subscriber->safe_psql('postgres', + "SET enable_seqscan = off; SET enable_bitmapscan = off; SELECT id FROM test_hot_excl WHERE k = 4294967296" +); +is($result, qq(1), 'and the exclusion index finds the row under its new value'); + +# A conflicting change is still caught by the subscriber's constraint. +my $log_offset = -s $node_subscriber->logfile; +$node_publisher->safe_psql('postgres', + "UPDATE test_hot_excl SET k = 4294967296 WHERE id = 2"); +$node_subscriber->wait_for_log( + qr/conflicting key value violates exclusion constraint "test_hot_excl_k_excl"/, + $log_offset); +pass('a conflicting applied UPDATE is rejected'); + +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_hot_excl"); +$node_subscriber->safe_psql('postgres', "DROP TABLE test_hot_excl"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_hot_excl"); +$node_publisher->safe_psql('postgres', "DROP TABLE test_hot_excl"); + +# Testcase end: an applied UPDATE can be HOT through an exclusion constraint +# ============================================================================= + $node_subscriber->stop('fast'); $node_publisher->stop('fast'); -- 2.50.1