From d74624c06baa831e12655a7b319b1fbd5dfb548b Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Mon, 19 Oct 2026 18:55:41 -0400 Subject: [PATCH v43 3/8] Allow HOT updates when expression and partial indexes are unaffected An UPDATE that changes a column used by an expression index cannot be HOT today, even when the expression's value does not change: an index on (data->>'name') blocks HOT for every change to data. Partial indexes are treated the same way, so an index WHERE status = 'active' blocks HOT for a row that is outside the predicate before and after the update. For JSONB documents with a few indexed fields that is most UPDATEs. Since the previous commit the executor computes which indexed attributes an UPDATE changed before calling the table AM, outside any buffer lock, so it can now look at what the indexes would store. INDEX_ATTR_BITMAP_EXPRESSION, a new relcache bitmap, holds the attributes that non-summarizing indexes reference only through expressions or as partial indexes. When the changed attributes are all in that set, or are referenced otherwise only by summarizing indexes, ExecUpdateModifiedIdxAttrs() evaluates each affected index's predicate and index datums for the old and new rows, and drops from the changed set every attribute whose change no index sees. The table AM decides the rest as before, so for heap the update can be HOT. An index sees the change if: - the row enters or leaves its predicate. A HOT chain would leave the index pointing at a row outside the predicate, which the planner relies on not happening; - the row is inside the predicate and any index datum differs. Datums are compared byte for byte as stored, using the type of the value FormIndexDatum() produced (the expression's type, not the index column's, which can differ through opckeytype), so equal values with different representations still count as changed; - the index is not yet ready for inserts or not valid. README.HOT requires such an index to constrain HOT, and a failed build's expression need not be computable for every row. The comparison is made per row, after BEFORE ROW triggers and after EvalPlanQual has replaced the old and new rows with those built from the latest version, so neither a concurrent update nor an earlier row of the same statement can supply the answer. update-epq-indexes, added with the tests, covers the concurrent case with a BEFORE ROW trigger. Nothing is evaluated when an update changes a column that a plain index uses, so the cost of an update that cannot be HOT is the attribute comparison it already paid. Key and replica identity indexes have neither expressions nor predicates, so the lock mode and the replica identity decision see the same attributes as before; an assertion checks this. This is the second attempt. Commit c203d6cf8 compared index datums using the index's storage type and crashed for an index whose stored type differs from the expression's; it was disabled in 5d28c9bd7 and reverted in 1c53c4dec. The hot_updates test now covers such indexes (name_ops, gist point_ops). Its expected output changes for the expression and partial index cases this makes HOT, and for no others. Discussion: https://postgr.es/m/78574B24-BE0A-42C5-8075-3FA9FA63B8FC@amazon.com Discussion: https://postgr.es/m/20181106185255.776mstcyehnc63ty@alvherre.pgsql # Conflicts: # contrib/pageinspect/expected/hot_updates.out # src/backend/executor/nodeModifyTable.c # src/backend/utils/cache/relcache.c # src/include/utils/rel.h # src/include/utils/relcache.h --- contrib/amcheck/expected/check_btree.out | 83 ++++++ contrib/amcheck/sql/check_btree.sql | 50 ++++ doc/src/sgml/storage.sgml | 9 +- src/backend/access/heap/README.HOT | 18 +- src/backend/access/heap/heapam.c | 14 +- src/backend/commands/repack.c | 3 +- src/backend/executor/execReplication.c | 3 +- src/backend/executor/nodeModifyTable.c | 273 +++++++++++++++++- src/backend/utils/cache/relcache.c | 46 +++ src/include/executor/executor.h | 3 +- src/include/nodes/execnodes.h | 8 + src/include/utils/rel.h | 2 + src/include/utils/relcache.h | 1 + .../isolation/expected/cic-expression-hot.out | 43 +++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/cic-expression-hot.spec | 43 +++ src/test/regress/expected/hot_updates.out | 26 +- 17 files changed, 599 insertions(+), 27 deletions(-) create mode 100644 src/test/isolation/expected/cic-expression-hot.out create mode 100644 src/test/isolation/specs/cic-expression-hot.spec diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index 4fcee5da463..6fc1ee97902 100644 --- a/contrib/amcheck/expected/check_btree.out +++ b/contrib/amcheck/expected/check_btree.out @@ -283,6 +283,89 @@ SELECT bt_index_check('varlena_bug_idx', true); (1 row) +-- HOT updates of rows indexed by expression and partial indexes must leave +-- every index entry consistent with the heap. +CREATE TABLE hot_expr (id int PRIMARY KEY, s text, d jsonb, + status text, v int, n numeric) WITH (fillfactor = 50); +CREATE INDEX hot_expr_lower ON hot_expr (lower(s)); +CREATE INDEX hot_expr_name ON hot_expr ((d->>'name')); +CREATE INDEX hot_expr_partial ON hot_expr (v) WHERE status = 'active'; +CREATE INDEX hot_expr_numeric ON hot_expr ((n + 0)); +INSERT INTO hot_expr + SELECT g, 'abc' || g, jsonb_build_object('name', 'n' || g, 'age', g), + CASE WHEN g % 2 = 0 THEN 'active' ELSE 'done' END, g, 3 + FROM generate_series(1, 200) g; +UPDATE hot_expr SET s = upper(s); +UPDATE hot_expr SET d = jsonb_set(d, '{age}', '0'); +UPDATE hot_expr SET v = v + 1000 WHERE status = 'done'; +UPDATE hot_expr SET status = 'done' WHERE id % 4 = 0; +UPDATE hot_expr SET status = 'active' WHERE id % 3 = 0; +UPDATE hot_expr SET n = 3.0 WHERE id % 5 = 0; +UPDATE hot_expr SET d = jsonb_set(d, '{name}', '"x"') WHERE id % 7 = 0; +SELECT bt_index_check('hot_expr_lower', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_check('hot_expr_name', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_check('hot_expr_partial', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_check('hot_expr_numeric', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_parent_check('hot_expr_partial', heapallindexed => true); + bt_index_parent_check +----------------------- + +(1 row) + +DROP TABLE hot_expr; +-- One UPDATE in which different rows are decided by different indexes: an +-- expression index created first, a partial index created after it. +CREATE TABLE hot_expr_multi (id int PRIMARY KEY, doc jsonb) WITH (fillfactor = 30); +CREATE INDEX hot_expr_multi_status ON hot_expr_multi ((doc->>'status')); +CREATE INDEX hot_expr_multi_vip ON hot_expr_multi ((doc->>'email')) + WHERE (doc->>'vip')::bool; +INSERT INTO hot_expr_multi + SELECT g, jsonb_build_object('status', 'a', 'email', 'e' || g, + 'vip', g % 2 = 0, 'n', 0) + FROM generate_series(1, 3000) g; +UPDATE hot_expr_multi SET doc = CASE + WHEN id % 4 = 0 THEN jsonb_set(doc, '{status}', '"b"') + WHEN id % 4 = 1 THEN jsonb_set(doc, '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + WHEN id % 4 = 2 THEN jsonb_set(jsonb_set(doc, '{status}', '"c"'), + '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + ELSE jsonb_set(doc, '{n}', '1') END; +UPDATE hot_expr_multi SET doc = CASE + WHEN id % 3 = 0 THEN jsonb_set(doc, '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + WHEN id % 3 = 1 THEN jsonb_set(doc, '{status}', '"d"') + ELSE jsonb_set(doc, '{n}', '2') END; +SELECT bt_index_check('hot_expr_multi_status', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_check('hot_expr_multi_vip', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +DROP TABLE hot_expr_multi; -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql index a5585085fad..4c54aad5d9f 100644 --- a/contrib/amcheck/sql/check_btree.sql +++ b/contrib/amcheck/sql/check_btree.sql @@ -183,6 +183,56 @@ INSERT INTO varlena_bug VALUES (repeat('Test', 250)); ALTER TABLE varlena_bug ALTER COLUMN v SET STORAGE extended; SELECT bt_index_check('varlena_bug_idx', true); +-- HOT updates of rows indexed by expression and partial indexes must leave +-- every index entry consistent with the heap. +CREATE TABLE hot_expr (id int PRIMARY KEY, s text, d jsonb, + status text, v int, n numeric) WITH (fillfactor = 50); +CREATE INDEX hot_expr_lower ON hot_expr (lower(s)); +CREATE INDEX hot_expr_name ON hot_expr ((d->>'name')); +CREATE INDEX hot_expr_partial ON hot_expr (v) WHERE status = 'active'; +CREATE INDEX hot_expr_numeric ON hot_expr ((n + 0)); +INSERT INTO hot_expr + SELECT g, 'abc' || g, jsonb_build_object('name', 'n' || g, 'age', g), + CASE WHEN g % 2 = 0 THEN 'active' ELSE 'done' END, g, 3 + FROM generate_series(1, 200) g; +UPDATE hot_expr SET s = upper(s); +UPDATE hot_expr SET d = jsonb_set(d, '{age}', '0'); +UPDATE hot_expr SET v = v + 1000 WHERE status = 'done'; +UPDATE hot_expr SET status = 'done' WHERE id % 4 = 0; +UPDATE hot_expr SET status = 'active' WHERE id % 3 = 0; +UPDATE hot_expr SET n = 3.0 WHERE id % 5 = 0; +UPDATE hot_expr SET d = jsonb_set(d, '{name}', '"x"') WHERE id % 7 = 0; +SELECT bt_index_check('hot_expr_lower', heapallindexed => true); +SELECT bt_index_check('hot_expr_name', heapallindexed => true); +SELECT bt_index_check('hot_expr_partial', heapallindexed => true); +SELECT bt_index_check('hot_expr_numeric', heapallindexed => true); +SELECT bt_index_parent_check('hot_expr_partial', heapallindexed => true); +DROP TABLE hot_expr; + +-- One UPDATE in which different rows are decided by different indexes: an +-- expression index created first, a partial index created after it. +CREATE TABLE hot_expr_multi (id int PRIMARY KEY, doc jsonb) WITH (fillfactor = 30); +CREATE INDEX hot_expr_multi_status ON hot_expr_multi ((doc->>'status')); +CREATE INDEX hot_expr_multi_vip ON hot_expr_multi ((doc->>'email')) + WHERE (doc->>'vip')::bool; +INSERT INTO hot_expr_multi + SELECT g, jsonb_build_object('status', 'a', 'email', 'e' || g, + 'vip', g % 2 = 0, 'n', 0) + FROM generate_series(1, 3000) g; +UPDATE hot_expr_multi SET doc = CASE + WHEN id % 4 = 0 THEN jsonb_set(doc, '{status}', '"b"') + WHEN id % 4 = 1 THEN jsonb_set(doc, '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + WHEN id % 4 = 2 THEN jsonb_set(jsonb_set(doc, '{status}', '"c"'), + '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + ELSE jsonb_set(doc, '{n}', '1') END; +UPDATE hot_expr_multi SET doc = CASE + WHEN id % 3 = 0 THEN jsonb_set(doc, '{vip}', to_jsonb(NOT (doc->>'vip')::bool)) + WHEN id % 3 = 1 THEN jsonb_set(doc, '{status}', '"d"') + ELSE jsonb_set(doc, '{n}', '2') END; +SELECT bt_index_check('hot_expr_multi_status', heapallindexed => true); +SELECT bt_index_check('hot_expr_multi_vip', heapallindexed => true); +DROP TABLE hot_expr_multi; + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 83de016eaa5..4d65523e7b9 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -1109,7 +1109,14 @@ data. Empty in ordinary tables. The update does not modify any columns referenced by the table's indexes, not including summarizing indexes. The only summarizing index method in the core PostgreSQL distribution is BRIN. + linkend="brin">BRIN. A column that indexes reference only through + expressions, or only as + partial indexes, does not count as + modified if the update changes neither the value of any such index's + expressions nor whether the row satisfies its predicate. The values are + compared as stored, so equal values with different representations, such + as 1.0 and 1.00 of type + numeric, count as different. diff --git a/src/backend/access/heap/README.HOT b/src/backend/access/heap/README.HOT index 74e407f375a..0377f508055 100644 --- a/src/backend/access/heap/README.HOT +++ b/src/backend/access/heap/README.HOT @@ -37,6 +37,14 @@ First, where a tuple is repeatedly updated in ways that do not change its indexed columns. (Here, "indexed column" means any column referenced at all in an index definition, including for example columns that are tested in a partial-index predicate but are not stored in the index.) +A column that indexes reference only through expressions, or only as +partial indexes, may change if the update changes nothing those indexes +store: not the value of any of their expressions or columns, as compared +byte for byte, and not whether the tuple satisfies their predicates. +The executor works that out before calling the table AM, and passes on +only the indexed columns whose change matters. 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. Second, where the modified columns are only used in indexes that do not contain tuple IDs, but maintain summaries of the indexed data by block. @@ -477,9 +485,10 @@ Heap-only tuple HOT-safe A proposed tuple update is said to be HOT-safe if it changes - none of the tuple's indexed columns. It will only become an - actual HOT update if we can find room on the same page for - the new tuple version. + none of the tuple's indexed columns, other than in ways the + indexes that use them cannot see (see above). It will only + become an actual HOT update if we can find room on the same page + for the new tuple version. HOT update @@ -496,7 +505,8 @@ Indexed column A column used in an index definition. The column might not actually be stored in the index --- it could be used in a functional index's expression, or used in a partial index - predicate. HOT treats all these cases alike. + predicate. A change to a column that indexes use only in these + ways blocks HOT only if it changes what those indexes store. Redirecting line pointer diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 729f9c5fb57..e0cd1ab7612 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3316,6 +3316,7 @@ heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup, bool rep_id_key_required = false; #ifdef USE_ASSERT_CHECKING Bitmapset *check_attrs; + Bitmapset *narrowable_attrs; #endif TransactionId xmax_new_tuple, xmax_old_tuple; @@ -3359,6 +3360,8 @@ heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup, id_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_IDENTITY_KEY); #ifdef USE_ASSERT_CHECKING check_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_INDEXED); + narrowable_attrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_EXPRESSION); #endif block = ItemPointerGetBlockNumber(otid); @@ -3417,6 +3420,7 @@ heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup, bms_free(id_attrs); #ifdef USE_ASSERT_CHECKING bms_free(check_attrs); + bms_free(narrowable_attrs); #endif /* modified_idx_attrs is owned by the caller, don't free it */ @@ -3765,6 +3769,7 @@ l2: bms_free(id_attrs); #ifdef USE_ASSERT_CHECKING bms_free(check_attrs); + bms_free(narrowable_attrs); #endif /* modified_idx_attrs is owned by the caller, don't free it */ @@ -3795,16 +3800,21 @@ l2: * updated, and no one else can have changed it. Check that every * attribute comparing the two tuples here finds changed is in the * caller's set, so that a caller that misses a change, or compares the - * wrong tuples, is caught. The caller may report more: comparing slots, - * it cannot always tell that two values will be stored alike. + * wrong tuples, is caught. The caller may leave out an attribute that + * indexes use only through expressions or as partial indexes, having + * found that no index's entry changes (see ExecUpdateModifiedIdxAttrs()), + * and may report more: comparing slots, it cannot always tell that two + * values will be stored alike. */ { Bitmapset *changed = HeapUpdateModifiedIdxAttrs(relation, check_attrs, &oldtup, newtup); + changed = bms_del_members(changed, narrowable_attrs); Assert(bms_is_subset(changed, modified_idx_attrs)); bms_free(changed); bms_free(check_attrs); + bms_free(narrowable_attrs); } #endif diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 5fc5808e079..4294c860e12 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2919,7 +2919,8 @@ apply_concurrent_update(Relation rel, TupleTableSlot *spilled_tuple, modified_idx_attrs = ExecUpdateModifiedIdxAttrs(chgcxt->cc_rri, ondisk_tuple, - spilled_tuple); + spilled_tuple, + chgcxt->cc_estate); /* * Carry out the update, skipping logical decoding for it. diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 5f894651d5e..64e2ca2297d 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -1029,7 +1029,8 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, ExecPartitionCheck(resultRelInfo, slot, estate, true); modified_idx_attrs = ExecUpdateModifiedIdxAttrs(resultRelInfo, - searchslot, slot); + searchslot, slot, + estate); simple_table_tuple_update_ext(rel, tid, slot, estate->es_snapshot, modified_idx_attrs, &update_indexes); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index d31eae94fb2..5a042eb83a3 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -59,6 +59,7 @@ #include "access/tupconvert.h" #include "access/tupdesc.h" #include "access/xact.h" +#include "catalog/index.h" #include "commands/trigger.h" #include "executor/execPartition.h" #include "executor/executor.h" @@ -75,6 +76,7 @@ #include "utils/builtins.h" #include "utils/datum.h" #include "utils/injection_point.h" +#include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/snapmgr.h" @@ -194,6 +196,148 @@ static TupleTableSlot *ExecMergeNotMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo, bool canSetTag); +/* + * Do the old and new index datums differ, byte for byte? Compare each + * column as the type FormIndexDatum() produced it in: the table column's + * type or the expression's, which need not be the type the index stores. + */ +static bool +IndexDatumsDiffer(Relation heapRel, IndexInfo *indexInfo, + const Datum *old_values, const bool *old_isnull, + const Datum *new_values, const bool *new_isnull) +{ + ListCell *expr = list_head(indexInfo->ii_Expressions); + + for (int i = 0; i < indexInfo->ii_NumIndexAttrs; i++) + { + AttrNumber attnum = indexInfo->ii_IndexAttrNumbers[i]; + int16 typlen; + bool typbyval; + + if (attnum != 0) + { + CompactAttribute *att = TupleDescCompactAttr(RelationGetDescr(heapRel), + attnum - 1); + + typlen = att->attlen; + typbyval = att->attbyval; + } + else + { + get_typlenbyval(exprType(lfirst(expr)), &typlen, &typbyval); + expr = lnext(indexInfo->ii_Expressions, expr); + } + + if (old_isnull[i] != new_isnull[i]) + return true; + if (!old_isnull[i] && + !datumIsEqual(old_values[i], new_values[i], typbyval, typlen)) + return true; + } + return false; +} + +/* + * Would the update from old_tts to new_tts give this index a new entry, or + * take one away? + * + * The index datums are formed in the per-tuple memory context, which + * callers reset for each row. The predicate's ExprState is kept in the + * IndexInfo, as ExecInsertIndexTuples() keeps it; ExecPrepareQual() builds + * it in the query context. + */ +static bool +IndexEntryChanges(ResultRelInfo *resultRelInfo, IndexInfo *indexInfo, + TupleTableSlot *old_tts, TupleTableSlot *new_tts, + EState *estate) +{ + ExprContext *econtext = GetPerTupleExprContext(estate); + TupleTableSlot *save_scantuple = econtext->ecxt_scantuple; + MemoryContext oldcxt; + Datum old_values[INDEX_MAX_KEYS]; + bool old_isnull[INDEX_MAX_KEYS]; + Datum new_values[INDEX_MAX_KEYS]; + bool new_isnull[INDEX_MAX_KEYS]; + bool result; + + if (indexInfo->ii_Predicate != NIL) + { + bool old_member; + bool new_member; + + if (indexInfo->ii_PredicateState == NULL) + indexInfo->ii_PredicateState = + ExecPrepareQual(indexInfo->ii_Predicate, estate); + + econtext->ecxt_scantuple = old_tts; + old_member = ExecQual(indexInfo->ii_PredicateState, econtext); + econtext->ecxt_scantuple = new_tts; + new_member = ExecQual(indexInfo->ii_PredicateState, econtext); + econtext->ecxt_scantuple = save_scantuple; + + /* + * A row that leaves or enters the index's predicate changes which + * rows the index holds. A row outside it on both sides is none of + * the index's business. + */ + if (old_member != new_member) + return true; + if (!old_member) + return false; + } + + oldcxt = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + econtext->ecxt_scantuple = old_tts; + FormIndexDatum(indexInfo, old_tts, estate, old_values, old_isnull); + econtext->ecxt_scantuple = new_tts; + FormIndexDatum(indexInfo, new_tts, estate, new_values, new_isnull); + econtext->ecxt_scantuple = save_scantuple; + + result = IndexDatumsDiffer(resultRelInfo->ri_RelationDesc, indexInfo, + old_values, old_isnull, new_values, new_isnull); + MemoryContextSwitchTo(oldcxt); + return result; +} + +/* + * Return the attributes of the i'th index that INDEX_ATTR_BITMAP_EXPRESSION + * can contain: those it references through its expressions, and for a + * partial index all those it references. Builds the per-index cache on + * first use. + */ +static Bitmapset * +IndexExprAttrs(ResultRelInfo *resultRelInfo, int i, EState *estate) +{ + if (resultRelInfo->ri_IndexExprAttrs == NULL) + { + MemoryContext oldcxt = MemoryContextSwitchTo(estate->es_query_cxt); + + resultRelInfo->ri_IndexExprAttrs = + palloc0_array(Bitmapset *, resultRelInfo->ri_NumIndices); + for (int j = 0; j < resultRelInfo->ri_NumIndices; j++) + { + IndexInfo *ii = resultRelInfo->ri_IndexRelationInfo[j]; + Bitmapset *attrs = NULL; + + pull_varattnos((Node *) ii->ii_Expressions, 1, &attrs); + if (ii->ii_Predicate != NIL) + { + pull_varattnos((Node *) ii->ii_Predicate, 1, &attrs); + for (int k = 0; k < ii->ii_NumIndexAttrs; k++) + { + if (ii->ii_IndexAttrNumbers[k] != 0) + attrs = bms_add_member(attrs, + ii->ii_IndexAttrNumbers[k] - + FirstLowInvalidHeapAttributeNumber); + } + } + resultRelInfo->ri_IndexExprAttrs[j] = attrs; + } + MemoryContextSwitchTo(oldcxt); + } + return resultRelInfo->ri_IndexExprAttrs[i]; +} + /* * ExecUpdateModifiedIdxAttrs * @@ -216,17 +360,137 @@ static TupleTableSlot *ExecMergeNotMatched(ModifyTableContext *context, * simple_heap_update() makes the same determination from a TID and a * HeapTuple in HeapUpdateModifiedIdxAttrs(), and simple_table_tuple_update() * with table_slot_modified_idx_attrs(). + * + * The set is then narrowed to what the indexes see, using estate. An + * attribute that non-summarizing indexes reference only through expressions + * or as partial indexes (INDEX_ATTR_BITMAP_EXPRESSION) is dropped from the + * set if, for every such index that references it, the update moves the row + * neither into nor out of the index's predicate and, if the row is in it, + * changes none of the index's datums. The table AM can then treat the + * update as if it had not changed the attribute: no index needs a new + * entry because of it. + * + * 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 + * one its expressions or predicate cannot be evaluated for; evaluating them + * would make the UPDATE fail. It still constrains HOT, as README.HOT + * requires. + * + * Narrowing never removes an attribute of a key or replica identity index, + * as those have neither expressions nor predicates, so the table AM may + * also take its lock mode and replica identity decisions from the result. */ Bitmapset * ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, TupleTableSlot *old_tts, - TupleTableSlot *new_tts) + TupleTableSlot *new_tts, + EState *estate) { + Relation relation = resultRelInfo->ri_RelationDesc; + Bitmapset *attrs; + Bitmapset *exprattrs; + Bitmapset *candidates; + if (resultRelInfo->ri_NumIndices == 0) return NULL; - return table_slot_modified_idx_attrs(resultRelInfo->ri_RelationDesc, - old_tts, new_tts); + attrs = table_slot_modified_idx_attrs(relation, old_tts, new_tts); + + if (bms_is_empty(attrs)) + return attrs; + + /* + * Narrowing can only make the update HOT if every changed attribute is + * either a candidate or referenced only by summarizing indexes. Otherwise + * some index needs a new entry anyway, and evaluating expressions would + * be wasted work. + */ + exprattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_EXPRESSION); + candidates = bms_intersect(attrs, exprattrs); + if (bms_is_empty(candidates)) + { + bms_free(exprattrs); + return attrs; + } + { + Bitmapset *rest = bms_difference(attrs, exprattrs); + Bitmapset *hotattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_HOT_BLOCKING); + bool hopeless = bms_overlap(rest, hotattrs); + + bms_free(rest); + bms_free(hotattrs); + bms_free(exprattrs); + if (hopeless) + { + bms_free(candidates); + return attrs; + } + } + + /* + * Keep each candidate that some index sees change. An index whose + * candidates are all kept already need not be asked. + */ + { + Bitmapset *keep = NULL; + + for (int i = 0; i < resultRelInfo->ri_NumIndices; i++) + { + IndexInfo *indexInfo = resultRelInfo->ri_IndexRelationInfo[i]; + Bitmapset *mine; + + if (resultRelInfo->ri_IndexRelationDescs[i] == NULL || + indexInfo->ii_Summarizing) + continue; + + mine = bms_intersect(IndexExprAttrs(resultRelInfo, i, estate), + candidates); + if (bms_is_empty(mine) || bms_is_subset(mine, keep)) + { + bms_free(mine); + continue; + } + + if (!indexInfo->ii_ReadyForInserts || + !resultRelInfo->ri_IndexRelationDescs[i]->rd_index->indisvalid || + IndexEntryChanges(resultRelInfo, indexInfo, + old_tts, new_tts, estate)) + { + keep = bms_add_members(keep, mine); + if (bms_is_subset(candidates, keep)) + { + bms_free(mine); + break; + } + } + bms_free(mine); + } + + candidates = bms_del_members(candidates, keep); + bms_free(keep); + } + +#ifdef USE_ASSERT_CHECKING + { + Bitmapset *keyattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_KEY); + Bitmapset *idattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + + Assert(!bms_overlap(candidates, keyattrs)); + Assert(!bms_overlap(candidates, idattrs)); + bms_free(keyattrs); + bms_free(idattrs); + } +#endif + + attrs = bms_del_members(attrs, candidates); + bms_free(candidates); + + return attrs; } /* @@ -2365,7 +2629,8 @@ lreplace: if (resultRelationDesc->rd_att->constr) ExecConstraints(resultRelInfo, slot, estate); - modified_idx_attrs = ExecUpdateModifiedIdxAttrs(resultRelInfo, oldSlot, slot); + modified_idx_attrs = ExecUpdateModifiedIdxAttrs(resultRelInfo, oldSlot, slot, + estate); /* * Call into the table AM to update the heap tuple. diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 7b83882bf28..c0941182b91 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -2478,6 +2478,7 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) bms_free(relation->rd_hotblockingattr); bms_free(relation->rd_summarizedattr); bms_free(relation->rd_indexedattr); + bms_free(relation->rd_exprattr); if (relation->rd_pubdesc) pfree(relation->rd_pubdesc); if (relation->rd_options) @@ -5327,6 +5328,9 @@ RelationGetIndexPredicate(Relation relation) * 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 any index + * INDEX_ATTR_BITMAP_EXPRESSION Columns that non-summarizing indexes + * reference only through expressions or + * as partial indexes * * Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that * we can include system attributes (e.g., OID) in the bitmap representation. @@ -5352,6 +5356,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind) Bitmapset *hotblockingattrs; /* columns with HOT blocking indexes */ Bitmapset *summarizedattrs; /* columns with summarizing indexes */ Bitmapset *indexedattrs; /* columns referenced by any index */ + Bitmapset *exprattrs; /* columns of expression or partial + * non-summarizing indexes */ + Bitmapset *plainattrs; /* columns of other non-summarizing indexes, + * and plain columns of expression indexes */ List *indexoidlist; List *newindexoidlist; Oid relpkindex; @@ -5376,6 +5384,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind) return bms_copy(relation->rd_summarizedattr); case INDEX_ATTR_BITMAP_INDEXED: return bms_copy(relation->rd_indexedattr); + case INDEX_ATTR_BITMAP_EXPRESSION: + return bms_copy(relation->rd_exprattr); default: elog(ERROR, "unknown attrKind %u", attrKind); } @@ -5420,6 +5430,8 @@ restart: idindexattrs = NULL; hotblockingattrs = NULL; summarizedattrs = NULL; + exprattrs = NULL; + plainattrs = NULL; foreach(l, indexoidlist) { Oid indexOid = lfirst_oid(l); @@ -5503,6 +5515,15 @@ restart: { *attrs = bms_add_member(*attrs, attrnum - FirstLowInvalidHeapAttributeNumber); + if (attrs == &hotblockingattrs) + { + if (indexPredicate != NULL) + exprattrs = bms_add_member(exprattrs, + attrnum - FirstLowInvalidHeapAttributeNumber); + else + plainattrs = bms_add_member(plainattrs, + attrnum - FirstLowInvalidHeapAttributeNumber); + } if (isKey && i < indexDesc->rd_index->indnkeyatts) uindexattrs = bms_add_member(uindexattrs, @@ -5524,6 +5545,12 @@ restart: /* Collect all attributes in the index predicate, too */ pull_varattnos(indexPredicate, 1, attrs); + if (attrs == &hotblockingattrs) + { + pull_varattnos(indexExpressions, 1, &exprattrs); + pull_varattnos(indexPredicate, 1, &exprattrs); + } + index_close(indexDesc, AccessShareLock); } @@ -5552,12 +5579,26 @@ restart: bms_free(idindexattrs); bms_free(hotblockingattrs); bms_free(summarizedattrs); + bms_free(exprattrs); + bms_free(plainattrs); goto restart; } indexedattrs = bms_union(hotblockingattrs, summarizedattrs); + /* + * Record what attributes non-summarizing indexes reference only through + * expressions, or only as partial indexes. A change to one of these + * needs new index entries only if it changes the value of an index's + * expressions or a row's membership in, or key within, a partial index. + * An attribute a summarizing index references is left out, as that index + * needs to see any change to it. + */ + exprattrs = bms_del_members(exprattrs, plainattrs); + exprattrs = bms_del_members(exprattrs, summarizedattrs); + bms_free(plainattrs); + /* Don't leak the old values of these bitmaps, if any */ relation->rd_attrsvalid = false; bms_free(relation->rd_keyattr); @@ -5572,6 +5613,8 @@ restart: relation->rd_summarizedattr = NULL; bms_free(relation->rd_indexedattr); relation->rd_indexedattr = NULL; + bms_free(relation->rd_exprattr); + relation->rd_exprattr = NULL; /* * Now save copies of the bitmaps in the relcache entry. We intentionally @@ -5587,6 +5630,7 @@ restart: relation->rd_hotblockingattr = bms_copy(hotblockingattrs); relation->rd_summarizedattr = bms_copy(summarizedattrs); relation->rd_indexedattr = bms_copy(indexedattrs); + relation->rd_exprattr = bms_copy(exprattrs); relation->rd_attrsvalid = true; MemoryContextSwitchTo(oldcxt); @@ -5605,6 +5649,8 @@ restart: return summarizedattrs; case INDEX_ATTR_BITMAP_INDEXED: return indexedattrs; + case INDEX_ATTR_BITMAP_EXPRESSION: + return exprattrs; default: elog(ERROR, "unknown attrKind %u", attrKind); return NULL; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 5cbdb092564..c82def87f2a 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -819,6 +819,7 @@ extern ResultRelInfo *ExecLookupResultRelByOid(ModifyTableState *node, bool update_cache); extern Bitmapset *ExecUpdateModifiedIdxAttrs(ResultRelInfo *relinfo, TupleTableSlot *old_tts, - TupleTableSlot *new_tts); + TupleTableSlot *new_tts, + EState *estate); #endif /* EXECUTOR_H */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index e3cea52e7df..220aaa8b965 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -502,6 +502,14 @@ typedef struct ResultRelInfo /* array of key/attr info for indices */ IndexInfo **ri_IndexRelationInfo; + /* + * For UPDATE, the table attributes each index references through + * expressions or its predicate, offset by + * FirstLowInvalidHeapAttributeNumber. Built on first use; NULL until + * then. + */ + Bitmapset **ri_IndexExprAttrs; + /* * 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/include/utils/rel.h b/src/include/utils/rel.h index 55b459adba8..fdd2a1e3213 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -165,6 +165,8 @@ typedef struct RelationData Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */ Bitmapset *rd_summarizedattr; /* cols indexed by summarizing indexes */ Bitmapset *rd_indexedattr; /* cols referenced by any index */ + Bitmapset *rd_exprattr; /* cols non-summarizing indexes reference only + * via expressions or as partial indexes */ PublicationDesc *rd_pubdesc; /* publication descriptor, or NULL */ diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 417990e8022..955d0186c2d 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -73,6 +73,7 @@ typedef enum IndexAttrBitmapKind INDEX_ATTR_BITMAP_HOT_BLOCKING, INDEX_ATTR_BITMAP_SUMMARIZED, INDEX_ATTR_BITMAP_INDEXED, + INDEX_ATTR_BITMAP_EXPRESSION, } IndexAttrBitmapKind; extern Bitmapset *RelationGetIndexAttrBitmap(Relation relation, diff --git a/src/test/isolation/expected/cic-expression-hot.out b/src/test/isolation/expected/cic-expression-hot.out new file mode 100644 index 00000000000..4128f40c141 --- /dev/null +++ b/src/test/isolation/expected/cic-expression-hot.out @@ -0,0 +1,43 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_lock s2_cic_s s1_upd_s s1_unlock s1_upd2 s1_check +step s1_lock: SELECT pg_advisory_lock(4242); +pg_advisory_lock +---------------- + +(1 row) + +step s2_cic_s: CREATE INDEX CONCURRENTLY cic_expr_idx ON cic_expr (cic_lower(s)); +step s1_upd_s: UPDATE cic_expr SET s = upper(s); +step s1_unlock: SELECT pg_advisory_unlock(4242); +pg_advisory_unlock +------------------ +t +(1 row) + +step s2_cic_s: <... completed> +step s1_upd2: UPDATE cic_expr SET s = lower(s) || ''; +step s1_check: + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT count(*) FROM cic_expr WHERE cic_lower(s) LIKE 'abc%'; + SELECT count(*) FROM cic_expr WHERE cic_lower(s) = 'abc42'; + RESET enable_seqscan; + RESET enable_bitmapscan; + SELECT indisvalid FROM pg_index WHERE indexrelid = 'cic_expr_idx'::regclass; + +count +----- + 100 +(1 row) + +count +----- + 1 +(1 row) + +indisvalid +---------- +t +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index d7da899aa85..2e99ca04305 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -95,6 +95,7 @@ test: skip-locked-3 test: skip-locked-4 test: drop-index-concurrently-1 test: multiple-cic +test: cic-expression-hot test: alter-domain-validate test: alter-table-1 test: alter-table-2 diff --git a/src/test/isolation/specs/cic-expression-hot.spec b/src/test/isolation/specs/cic-expression-hot.spec new file mode 100644 index 00000000000..ad1b612be01 --- /dev/null +++ b/src/test/isolation/specs/cic-expression-hot.spec @@ -0,0 +1,43 @@ +# UPDATEs during CREATE INDEX CONCURRENTLY of an expression index. +# +# The build waits for an advisory lock that s1 holds, inside the index +# expression, so s1's UPDATE runs while the index is not yet ready for +# inserts. The update leaves the expression's value alone; the finished +# index must find every row under its current key. + +setup +{ + CREATE TABLE cic_expr (id int PRIMARY KEY, s text, x int) + WITH (fillfactor = 50); + INSERT INTO cic_expr SELECT g, 'abc' || g, g FROM generate_series(1, 100) g; + CREATE FUNCTION cic_lower(text) RETURNS text IMMUTABLE LANGUAGE plpgsql AS $$ + BEGIN PERFORM pg_advisory_lock_shared(4242); PERFORM pg_advisory_unlock_shared(4242); RETURN lower($1); END; + $$; +} + +teardown +{ + DROP TABLE cic_expr; + DROP FUNCTION cic_lower(text); +} + +session s1 +step s1_lock { SELECT pg_advisory_lock(4242); } +step s1_upd_s { UPDATE cic_expr SET s = upper(s); } +step s1_unlock { SELECT pg_advisory_unlock(4242); } +step s1_upd2 { UPDATE cic_expr SET s = lower(s) || ''; } +step s1_check +{ + SET enable_seqscan = off; + SET enable_bitmapscan = off; + SELECT count(*) FROM cic_expr WHERE cic_lower(s) LIKE 'abc%'; + SELECT count(*) FROM cic_expr WHERE cic_lower(s) = 'abc42'; + RESET enable_seqscan; + RESET enable_bitmapscan; + SELECT indisvalid FROM pg_index WHERE indexrelid = 'cic_expr_idx'::regclass; +} + +session s2 +step s2_cic_s { CREATE INDEX CONCURRENTLY cic_expr_idx ON cic_expr (cic_lower(s)); } + +permutation s1_lock s2_cic_s s1_upd_s s1_unlock s1_upd2 s1_check diff --git a/src/test/regress/expected/hot_updates.out b/src/test/regress/expected/hot_updates.out index f83a686f3a9..276a845a3e9 100644 --- a/src/test/regress/expected/hot_updates.out +++ b/src/test/regress/expected/hot_updates.out @@ -321,7 +321,7 @@ SELECT pg_stat_force_next_flush(); SELECT * FROM get_hot_count('hot_jsonb_test'); updates | hot ---------+----- - 1 | 0 + 1 | 1 (1 row) UPDATE hot_jsonb_test SET data = data - 'city' WHERE id = 2; @@ -334,7 +334,7 @@ SELECT pg_stat_force_next_flush(); SELECT * FROM get_hot_count('hot_jsonb_test'); updates | hot ---------+----- - 2 | 0 + 2 | 2 (1 row) UPDATE hot_jsonb_test SET data = jsonb_insert(data, '{country}', '"USA"') WHERE id = 2; @@ -347,7 +347,7 @@ SELECT pg_stat_force_next_flush(); SELECT * FROM get_hot_count('hot_jsonb_test'); updates | hot ---------+----- - 3 | 0 + 3 | 3 (1 row) DROP TABLE hot_jsonb_test; @@ -379,7 +379,7 @@ SELECT * FROM update_case( NOTICE: table "hot_expr_test" does not exist, skipping updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) SELECT * FROM update_case( @@ -389,7 +389,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET a = a - 1, b = b + 1$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- Expression value changed. @@ -424,7 +424,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET s = upper(s)$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) SELECT * FROM update_case( @@ -434,7 +434,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET p = point(p[0], p[1] + 1)$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- An expression that is a point, stored by the gist index as a box. @@ -447,7 +447,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET d = d + 7$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- A plain index on the same column as an expression index. @@ -472,7 +472,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET s = upper(s), b = b + 1$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- A BEFORE ROW trigger that changes the expression's value. @@ -501,7 +501,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET v = v + 1$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) SELECT * FROM update_case( @@ -511,7 +511,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET status = 'archived'$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- Both versions inside the predicate with the key unchanged. @@ -522,7 +522,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET status = 'b'$$); updated | hot ---------+----- - 10 | 0 + 10 | 10 (1 row) -- Entering or leaving the predicate, or changing the key inside it. @@ -565,7 +565,7 @@ SELECT * FROM update_case( $$UPDATE hot_expr_test SET v = v + 100 WHERE status = 'done'$$); updated | hot ---------+----- - 5 | 0 + 5 | 5 (1 row) SET enable_seqscan = off; -- 2.50.1