From b3c502ad2a2300ac42a8b3c39605c3d846c3eae8 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Sun, 18 Oct 2026 17:48:19 -0400 Subject: [PATCH v41 10/10] Reuse an UPDATE's computed index datums when inserting its entries To decide whether an UPDATE can leave an index's entries alone, ExecUpdateModifiedIdxAttrs() evaluates the index's predicate and forms its datums for the old row and the new. When some index needs a new entry anyway, ExecInsertIndexTuples() then evaluated the predicate and formed the datums for the new row again. For an expensive expression whose value changes (to_tsvector() or md5() of a large text) that third evaluation made the UPDATE about half again as slow as before these commits; Matthias van de Meent suggested passing the datums on. Save, per index, what ExecUpdateModifiedIdxAttrs() learned of the new row in the ResultRelInfo: that it is outside the index's predicate, or its datums, which stay in the per-tuple memory context until the next row. ExecInsertIndexTuples() uses them when its caller passes EIIT_NEW_DATUMS, which the UPDATE, logical replication apply and REPACK paths do, and ExecCheckHotExclusion() always does. Indexes that were not evaluated are evaluated as before. The saved values describe the row ExecUpdateModifiedIdxAttrs() decided last. It resets them first, and an EvalPlanQual retry decides the row again; a cross-partition update inserts through another ResultRelInfo without the flag. Builds with assertions recompute each reused value and check that it matches. An expression whose value changes is now evaluated twice per row, for the old and new rows, as against once before these commits; there is no way to know the value is unchanged without the old row's. Tests in check_btree cover reuse with a partial index the row stays in and one it leaves. Saving the old row's datums instead of the new row's, or keeping one row's datums for the next, makes them fail. --- contrib/amcheck/expected/check_btree.out | 52 +++++++++ contrib/amcheck/sql/check_btree.sql | 28 +++++ src/backend/access/heap/README.HOT | 4 +- src/backend/commands/repack.c | 2 +- src/backend/executor/execIndexing.c | 136 ++++++++++++++++++----- src/backend/executor/execReplication.c | 2 +- src/backend/executor/nodeModifyTable.c | 57 ++++++++-- src/include/executor/executor.h | 20 ++++ src/include/nodes/execnodes.h | 11 ++ src/tools/pgindent/typedefs.list | 2 + 10 files changed, 274 insertions(+), 40 deletions(-) diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index 4e02954b6f8..5508e5046b7 100644 --- a/contrib/amcheck/expected/check_btree.out +++ b/contrib/amcheck/expected/check_btree.out @@ -464,6 +464,58 @@ SELECT bt_index_check('hot_bt_image_e', heapallindexed => true); DROP TABLE hot_bt_image; DROP FUNCTION hot_bt_image_count; DROP FUNCTION hot_bt_image_plan; +-- An UPDATE that evaluates an index for the old and new rows, to find out +-- whether its entry changes, inserts the new row's entry with the values +-- already computed, when it inserts one. Here hot_ev_part's predicate uses +-- x, so a change to x evaluates it, and hot_ev_x's change forces new +-- entries; and the second update also takes rows out of hot_ev_part. The +-- indexes must be right either way. (Builds with assertions check each +-- reused value against a fresh evaluation.) +CREATE FUNCTION hot_ev(t text) RETURNS text LANGUAGE sql IMMUTABLE + RETURN upper(t); +CREATE TABLE hot_ev_tab (id int PRIMARY KEY, t text, x int) WITH (fillfactor = 50); +CREATE INDEX hot_ev_t ON hot_ev_tab (hot_ev(t)); +CREATE INDEX hot_ev_part ON hot_ev_tab (hot_ev(t)) WHERE x >= 0; +CREATE INDEX hot_ev_x ON hot_ev_tab (x); +INSERT INTO hot_ev_tab SELECT g, 'a' || g, 0 FROM generate_series(1, 10) g; +UPDATE hot_ev_tab SET x = x + 1; +UPDATE hot_ev_tab SET t = t || 'y', x = -1 WHERE id <= 5; +SELECT bt_index_check('hot_ev_t', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SELECT bt_index_check('hot_ev_part', heapallindexed => true); + bt_index_check +---------------- + +(1 row) + +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT id FROM hot_ev_tab WHERE hot_ev(t) = 'A1Y'; + id +---- + 1 +(1 row) + +SELECT id FROM hot_ev_tab WHERE hot_ev(t) = 'A9' AND x >= 0; + id +---- + 9 +(1 row) + +SELECT count(*) FROM hot_ev_tab WHERE hot_ev(t) LIKE 'A%' AND x >= 0; + count +------- + 5 +(1 row) + +RESET enable_seqscan; +RESET enable_bitmapscan; +DROP TABLE hot_ev_tab; +DROP FUNCTION hot_ev; -- 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 0e03eeef9e0..bd9b97707a3 100644 --- a/contrib/amcheck/sql/check_btree.sql +++ b/contrib/amcheck/sql/check_btree.sql @@ -279,6 +279,34 @@ DROP TABLE hot_bt_image; DROP FUNCTION hot_bt_image_count; DROP FUNCTION hot_bt_image_plan; +-- An UPDATE that evaluates an index for the old and new rows, to find out +-- whether its entry changes, inserts the new row's entry with the values +-- already computed, when it inserts one. Here hot_ev_part's predicate uses +-- x, so a change to x evaluates it, and hot_ev_x's change forces new +-- entries; and the second update also takes rows out of hot_ev_part. The +-- indexes must be right either way. (Builds with assertions check each +-- reused value against a fresh evaluation.) +CREATE FUNCTION hot_ev(t text) RETURNS text LANGUAGE sql IMMUTABLE + RETURN upper(t); +CREATE TABLE hot_ev_tab (id int PRIMARY KEY, t text, x int) WITH (fillfactor = 50); +CREATE INDEX hot_ev_t ON hot_ev_tab (hot_ev(t)); +CREATE INDEX hot_ev_part ON hot_ev_tab (hot_ev(t)) WHERE x >= 0; +CREATE INDEX hot_ev_x ON hot_ev_tab (x); +INSERT INTO hot_ev_tab SELECT g, 'a' || g, 0 FROM generate_series(1, 10) g; +UPDATE hot_ev_tab SET x = x + 1; +UPDATE hot_ev_tab SET t = t || 'y', x = -1 WHERE id <= 5; +SELECT bt_index_check('hot_ev_t', heapallindexed => true); +SELECT bt_index_check('hot_ev_part', heapallindexed => true); +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT id FROM hot_ev_tab WHERE hot_ev(t) = 'A1Y'; +SELECT id FROM hot_ev_tab WHERE hot_ev(t) = 'A9' AND x >= 0; +SELECT count(*) FROM hot_ev_tab WHERE hot_ev(t) LIKE 'A%' AND x >= 0; +RESET enable_seqscan; +RESET enable_bitmapscan; +DROP TABLE hot_ev_tab; +DROP FUNCTION hot_ev; + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; diff --git a/src/backend/access/heap/README.HOT b/src/backend/access/heap/README.HOT index 44f0da658eb..34bbf376e67 100644 --- a/src/backend/access/heap/README.HOT +++ b/src/backend/access/heap/README.HOT @@ -46,7 +46,9 @@ 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 or replica identity. The executor works that out before calling the table AM, and passes on only the -indexed columns whose change matters. +indexed columns whose change matters. If the update then needs new index +entries anyway, the new tuple's entries are made from the values the +executor computed for it, not evaluated again. 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 diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 359499ec1d7..6bc4c6baaba 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2942,7 +2942,7 @@ apply_concurrent_update(Relation rel, TupleTableSlot *spilled_tuple, if (update_indexes != TU_None) { - uint32 flags = EIIT_IS_UPDATE; + uint32 flags = EIIT_IS_UPDATE | EIIT_NEW_DATUMS; if (update_indexes == TU_Summarizing) flags |= EIIT_ONLY_SUMMARIZING; diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index 532a73cfdbe..1ab7a04d47d 100644 --- a/src/backend/executor/execIndexing.c +++ b/src/backend/executor/execIndexing.c @@ -114,6 +114,7 @@ #include "executor/executor.h" #include "nodes/nodeFuncs.h" #include "storage/lmgr.h" +#include "utils/datum.h" #include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/multirangetypes.h" @@ -140,6 +141,9 @@ 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 FormIndexDatumForRow(ResultRelInfo *resultRelInfo, int i, + bool reuse, TupleTableSlot *slot, + EState *estate, Datum *values, bool *isnull); static bool check_exclusion_for_insert(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, @@ -276,6 +280,96 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo) */ } +/* + * Does the i'th index get an entry for the row in slot, and with which + * datums? If the index is partial and the row does not satisfy its + * predicate, return false. Otherwise form the datums into values and + * isnull, and return true. + * + * With reuse, slot must hold the new row of the UPDATE that + * ExecUpdateModifiedIdxAttrs() decided last: what that found out about + * this index for the row, if anything, is used instead of evaluating the + * index's predicate and expressions again. + */ +static bool +FormIndexDatumForRow(ResultRelInfo *resultRelInfo, int i, bool reuse, + TupleTableSlot *slot, EState *estate, + Datum *values, bool *isnull) +{ + IndexInfo *indexInfo = resultRelInfo->ri_IndexRelationInfo[i]; + ExprContext *econtext = GetPerTupleExprContext(estate); + NewIndexDatums *nid = NULL; + + if (reuse && resultRelInfo->ri_NewIndexDatums != NULL) + nid = &resultRelInfo->ri_NewIndexDatums[i]; + + if (nid != NULL && nid->state == NEWIDX_OUTSIDE) + { + Assert(indexInfo->ii_Predicate != NIL); + return false; + } + if (nid != NULL && nid->state == NEWIDX_DATUMS) + { + memcpy(values, nid->values, + sizeof(Datum) * indexInfo->ii_NumIndexAttrs); + memcpy(isnull, nid->isnull, + sizeof(bool) * indexInfo->ii_NumIndexAttrs); +#ifdef USE_ASSERT_CHECKING + { + Datum check_values[INDEX_MAX_KEYS]; + bool check_isnull[INDEX_MAX_KEYS]; + ListCell *expr = list_head(indexInfo->ii_Expressions); + + /* the saved datums must be what evaluating would give */ + econtext->ecxt_scantuple = slot; + if (indexInfo->ii_Predicate != NIL) + { + if (indexInfo->ii_PredicateState == NULL) + indexInfo->ii_PredicateState = + ExecPrepareQual(indexInfo->ii_Predicate, estate); + Assert(ExecQual(indexInfo->ii_PredicateState, econtext)); + } + FormIndexDatum(indexInfo, slot, estate, + check_values, check_isnull); + for (int k = 0; k < indexInfo->ii_NumIndexAttrs; k++) + { + AttrNumber attnum = indexInfo->ii_IndexAttrNumbers[k]; + Oid typid; + int16 typlen; + bool typbyval; + + if (attnum != 0) + typid = TupleDescAttr(RelationGetDescr(resultRelInfo->ri_RelationDesc), + attnum - 1)->atttypid; + else + { + typid = exprType(lfirst(expr)); + expr = lnext(indexInfo->ii_Expressions, expr); + } + get_typlenbyval(typid, &typlen, &typbyval); + Assert(check_isnull[k] == isnull[k]); + Assert(isnull[k] || + datumIsEqual(check_values[k], values[k], + typbyval, typlen)); + } + } +#endif + return true; + } + + econtext->ecxt_scantuple = slot; + if (indexInfo->ii_Predicate != NIL) + { + if (indexInfo->ii_PredicateState == NULL) + indexInfo->ii_PredicateState = + ExecPrepareQual(indexInfo->ii_Predicate, estate); + if (!ExecQual(indexInfo->ii_PredicateState, econtext)) + return false; + } + FormIndexDatum(indexInfo, slot, estate, values, isnull); + return true; +} + /* ---------------------------------------------------------------- * ExecInsertIndexTuples * @@ -382,36 +476,15 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, if ((flags & EIIT_ONLY_SUMMARIZING) && !indexInfo->ii_Summarizing) continue; - /* Check for partial index */ - if (indexInfo->ii_Predicate != NIL) - { - ExprState *predicate; - - /* - * If predicate state not set up yet, create it (in the estate's - * per-query context) - */ - predicate = indexInfo->ii_PredicateState; - if (predicate == NULL) - { - predicate = ExecPrepareQual(indexInfo->ii_Predicate, estate); - indexInfo->ii_PredicateState = predicate; - } - - /* Skip this index-update if the predicate isn't satisfied */ - if (!ExecQual(predicate, econtext)) - continue; - } - /* - * FormIndexDatum fills in its values and isnull parameters with the - * appropriate values for the column(s) of the index. + * Skip a partial index whose predicate the row does not satisfy, and + * form the index's datums for the row. For an UPDATE, these may be + * known already. */ - FormIndexDatum(indexInfo, - slot, - estate, - values, - isnull); + if (!FormIndexDatumForRow(resultRelInfo, i, + (flags & EIIT_NEW_DATUMS) != 0, + slot, estate, values, isnull)) + continue; /* Check whether to apply noDupErr to this index */ applyNoDupErr = (flags & EIIT_NO_DUPE_ERROR) && @@ -591,9 +664,12 @@ ExecCheckHotExclusion(ResultRelInfo *resultRelInfo, EState *estate, /* * ExecUpdateModifiedIdxAttrs() only notes an index the row is in, so - * a partial index's predicate holds. + * a partial index's predicate holds, and has the row's datums. */ - FormIndexDatum(indexInfo, slot, estate, values, isnull); + if (!FormIndexDatumForRow(resultRelInfo, i, true, slot, estate, + values, isnull)) + elog(ERROR, "row outside the predicate of index \"%s\"", + RelationGetRelationName(indexRelation)); applyNoDupErr = (flags & EIIT_NO_DUPE_ERROR) && (arbiterIndexes == NIL || diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 6b7d0d5e62b..61aeda97db0 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -982,7 +982,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_NumIndices > 0 && (update_indexes != TU_None)) { - uint32 flags = EIIT_IS_UPDATE; + uint32 flags = EIIT_IS_UPDATE | EIIT_NEW_DATUMS; if (conflictindexes != NIL) flags |= EIIT_NO_DUPE_ERROR; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ffe3a6d8b65..0a1e735441a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -260,16 +260,51 @@ IndexDatumsDiffer(Relation heapRel, Relation indexRel, IndexInfo *indexInfo, } /* - * Would the update from old_tts to new_tts give this index a new entry, or - * take one away? + * Remember what was found out about the i'th index for the new row, for + * FormIndexDatumForRow() in execIndexing.c to use. + */ +static void +SaveNewIndexDatums(ResultRelInfo *resultRelInfo, int i, EState *estate, + NewIndexDatumsState state, + const Datum *values, const bool *isnull) +{ + NewIndexDatums *nid; + int natts = resultRelInfo->ri_IndexRelationInfo[i]->ii_NumIndexAttrs; + + if (resultRelInfo->ri_NewIndexDatums == NULL) + resultRelInfo->ri_NewIndexDatums = + MemoryContextAllocZero(estate->es_query_cxt, + sizeof(NewIndexDatums) * + resultRelInfo->ri_NumIndices); + nid = &resultRelInfo->ri_NewIndexDatums[i]; + nid->state = state; + if (state != NEWIDX_DATUMS) + return; + if (nid->values == NULL) + { + nid->values = MemoryContextAlloc(estate->es_query_cxt, + sizeof(Datum) * natts); + nid->isnull = MemoryContextAlloc(estate->es_query_cxt, + sizeof(bool) * natts); + } + /* by-reference datums still point into per-tuple memory */ + memcpy(nid->values, values, sizeof(Datum) * natts); + memcpy(nid->isnull, isnull, sizeof(bool) * natts); +} + +/* + * Would the update from old_tts to new_tts give the i'th 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. + * it in the query context. What is learned of the new row is saved, so + * that inserting its index entries need not evaluate the index's + * expressions or predicate again. */ static bool -IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, +IndexEntryChanges(ResultRelInfo *resultRelInfo, int i, Relation indexRel, IndexInfo *indexInfo, TupleTableSlot *old_tts, TupleTableSlot *new_tts, EState *estate, bool *valchanged) { @@ -303,6 +338,9 @@ IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, * rows the index holds. A row outside it on both sides is none of * the index's business. */ + if (!new_member) + SaveNewIndexDatums(resultRelInfo, i, estate, NEWIDX_OUTSIDE, + NULL, NULL); if (old_member != new_member) return true; if (!old_member) @@ -320,6 +358,8 @@ IndexEntryChanges(ResultRelInfo *resultRelInfo, Relation indexRel, indexInfo, old_values, old_isnull, new_values, new_isnull, valchanged); MemoryContextSwitchTo(oldcxt); + SaveNewIndexDatums(resultRelInfo, i, estate, NEWIDX_DATUMS, + new_values, new_isnull); return result; } @@ -410,7 +450,7 @@ IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, { bool changed; - if (IndexEntryChanges(resultRelInfo, + if (IndexEntryChanges(resultRelInfo, i, resultRelInfo->ri_IndexRelationDescs[i], indexInfo, old_tts, new_tts, estate, &changed) || changed) @@ -423,7 +463,7 @@ IndexUnaffected(ResultRelInfo *resultRelInfo, int i, Bitmapset *mine, return true; #endif - return !IndexEntryChanges(resultRelInfo, + return !IndexEntryChanges(resultRelInfo, i, resultRelInfo->ri_IndexRelationDescs[i], indexInfo, old_tts, new_tts, estate, valchanged); @@ -500,6 +540,9 @@ ExecUpdateModifiedIdxAttrs(ResultRelInfo *resultRelInfo, /* what follows describes this row */ bms_free(resultRelInfo->ri_HotExclusionCheck); resultRelInfo->ri_HotExclusionCheck = NULL; + if (resultRelInfo->ri_NewIndexDatums != NULL) + for (int i = 0; i < resultRelInfo->ri_NumIndices; i++) + resultRelInfo->ri_NewIndexDatums[i].state = NEWIDX_UNKNOWN; if (resultRelInfo->ri_NumIndices == 0) return NULL; @@ -2969,7 +3012,7 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, /* insert index entries for tuple if necessary */ if (resultRelInfo->ri_NumIndices > 0 && (updateCxt->updateIndexes != TU_None)) { - uint32 flags = EIIT_IS_UPDATE; + uint32 flags = EIIT_IS_UPDATE | EIIT_NEW_DATUMS; if (updateCxt->updateIndexes == TU_Summarizing) flags |= EIIT_ONLY_SUMMARIZING; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index a8217593b6c..928570f2ebd 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -756,6 +756,26 @@ extern void ExecCloseIndices(ResultRelInfo *resultRelInfo); #define EIIT_IS_UPDATE (1<<0) #define EIIT_NO_DUPE_ERROR (1<<1) #define EIIT_ONLY_SUMMARIZING (1<<2) +#define EIIT_NEW_DATUMS (1<<3) /* use ri_NewIndexDatums */ + +/* + * What ExecUpdateModifiedIdxAttrs() found out about one index for the new + * row of an UPDATE (see ri_NewIndexDatums). + */ +typedef enum NewIndexDatumsState +{ + NEWIDX_UNKNOWN, /* not evaluated for this row */ + NEWIDX_OUTSIDE, /* new row does not satisfy the predicate */ + NEWIDX_DATUMS, /* new row's datums are in values/isnull */ +} NewIndexDatumsState; + +typedef struct NewIndexDatums +{ + NewIndexDatumsState state; + Datum *values; /* ii_NumIndexAttrs entries */ + bool *isnull; +} NewIndexDatums; + extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, EState *estate, uint32 flags, TupleTableSlot *slot, List *arbiterIndexes, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 38d6caa44cf..f68d61e0142 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -534,6 +534,17 @@ typedef struct ResultRelInfo */ Bitmapset *ri_HotExclusionCheck; + /* + * For UPDATE, per index (NULL until first needed), what + * ExecUpdateModifiedIdxAttrs() learned of the new row while deciding the + * row's update: whether it satisfies the index's predicate, and its index + * datums. Inserting the new row's index entries, and checking an + * exclusion constraint after a HOT update, use these instead of + * evaluating the index's expressions and predicate again. The datums are + * in the per-tuple memory context, so they are valid until the next row. + */ + struct NewIndexDatums *ri_NewIndexDatums; + /* * 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/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 16e12f0854f..c293c2c429e 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1818,6 +1818,8 @@ NestLoopState NestPath NewColumnValue NewConstraint +NewIndexDatums +NewIndexDatumsState NextSampleBlock_function NextSampleTuple_function NextValueExpr -- 2.50.1