From acae29b395291cb357ea143f2a89c8e8b5e9d437 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 29 Dec 2020 15:14:35 -0800 Subject: [PATCH v12 1/2] Pass down "logically unchanged index" hint. Add an executor aminsert() hinting mechanism that informs index AMs that the incoming index tuple (the tuple that accompanies the hint) is not being inserted because of a logical change to indexed columns (from an UPDATE or INSERT statement). This "logically unchanged" hint indicates that the incoming item must be a duplicate of some existing item in the index (a physical tuple that represents an obsolescent version of the logical row affected by an UPDATE). When this happens the new tuple is only needed so that the index will have a separate entry for the latest logical row (its latest version/HOT chain). An aminsert() call that uses the hint is fundamentally different to any aminsert() call needed for an INSERT statement. It's also fundamentally different to an aminsert() call needed for an UPDATE statement where the index is logically changed by the UPDATE. Recognizing this difference can help with cleanup of garbage index tuples in index access methods. Cleanup can intelligently target tuples that are likely to be garbage, without wasting too many cycles on less promising tuples/pages (pages with little or no version churn). This commit is infrastructure for an upcoming commit that will teach nbtree to perform bottom-up index deletion. No index AM applies the hint at this point. It is not useful on its own. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=CEKFa74EScx_hFVshCOn6AA5T-ajFASTdzipdkLTNQQ@mail.gmail.com --- src/include/access/amapi.h | 1 + src/include/access/brin_internal.h | 1 + src/include/access/genam.h | 1 + src/include/access/gin_private.h | 1 + src/include/access/gist_private.h | 1 + src/include/access/hash.h | 1 + src/include/access/nbtree.h | 1 + src/include/access/spgist.h | 1 + src/include/executor/executor.h | 1 + src/backend/access/brin/brin.c | 1 + src/backend/access/common/toast_internals.c | 2 +- src/backend/access/gin/gininsert.c | 1 + src/backend/access/gist/gist.c | 1 + src/backend/access/hash/hash.c | 1 + src/backend/access/heap/heapam_handler.c | 1 + src/backend/access/index/indexam.c | 4 +- src/backend/access/nbtree/nbtree.c | 1 + src/backend/access/spgist/spginsert.c | 1 + src/backend/catalog/indexing.c | 1 + src/backend/commands/constraint.c | 2 +- src/backend/commands/copyfrom.c | 5 +- src/backend/commands/trigger.c | 6 +- src/backend/executor/execIndexing.c | 160 +++++++++++++++++- src/backend/executor/execMain.c | 8 +- src/backend/executor/execReplication.c | 8 +- src/backend/executor/nodeModifyTable.c | 6 +- src/backend/replication/logical/worker.c | 3 +- contrib/bloom/blinsert.c | 1 + contrib/bloom/bloom.h | 1 + doc/src/sgml/indexam.sgml | 15 ++ .../modules/dummy_index_am/dummy_index_am.c | 1 + 31 files changed, 214 insertions(+), 25 deletions(-) diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h index 85b4766016..499119468d 100644 --- a/src/include/access/amapi.h +++ b/src/include/access/amapi.h @@ -110,6 +110,7 @@ typedef bool (*aminsert_function) (Relation indexRelation, ItemPointer heap_tid, Relation heapRelation, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); /* bulk delete */ diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index 9ffc9100c0..74a0e7eb29 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -91,6 +91,7 @@ extern void brinbuildempty(Relation index); extern bool brininsert(Relation idxRel, Datum *values, bool *nulls, ItemPointer heaptid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern IndexScanDesc brinbeginscan(Relation r, int nkeys, int norderbys); extern int64 bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm); diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 68d90f5141..bf2c2278d9 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -143,6 +143,7 @@ extern bool index_insert(Relation indexRelation, ItemPointer heap_t_ctid, Relation heapRelation, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern IndexScanDesc index_beginscan(Relation heapRelation, diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index 5cb2f72e4c..ff60f5708e 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -116,6 +116,7 @@ extern void ginbuildempty(Relation index); extern bool gininsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern void ginEntryInsert(GinState *ginstate, OffsetNumber attnum, Datum key, GinNullCategory category, diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index b68c01a5f2..09f9ad6d95 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -403,6 +403,7 @@ extern void gistbuildempty(Relation index); extern bool gistinsert(Relation r, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern MemoryContext createTempGistContext(void); extern GISTSTATE *initGISTstate(Relation index); diff --git a/src/include/access/hash.h b/src/include/access/hash.h index bab4d9f1b0..d5e65ff2be 100644 --- a/src/include/access/hash.h +++ b/src/include/access/hash.h @@ -364,6 +364,7 @@ extern void hashbuildempty(Relation index); extern bool hashinsert(Relation rel, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern bool hashgettuple(IndexScanDesc scan, ScanDirection dir); extern int64 hashgetbitmap(IndexScanDesc scan, TIDBitmap *tbm); diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index e8fecc6026..3b60e696eb 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -996,6 +996,7 @@ extern void btbuildempty(Relation index); extern bool btinsert(Relation rel, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern IndexScanDesc btbeginscan(Relation rel, int nkeys, int norderbys); extern Size btestimateparallelscan(void); diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h index 9f2ccc1730..544f240d1f 100644 --- a/src/include/access/spgist.h +++ b/src/include/access/spgist.h @@ -199,6 +199,7 @@ extern void spgbuildempty(Relation index); extern bool spginsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); /* spgscan.c */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 0c48d2a519..c8185bf8ce 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -581,6 +581,7 @@ extern void ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative); extern void ExecCloseIndices(ResultRelInfo *resultRelInfo); extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, + bool update, bool noDupErr, bool *specConflict, List *arbiterIndexes); extern bool ExecCheckIndexConstraints(ResultRelInfo *resultRelInfo, diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 1f72562c60..bea078944c 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -151,6 +151,7 @@ bool brininsert(Relation idxRel, Datum *values, bool *nulls, ItemPointer heaptid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { BlockNumber pagesPerRange; diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c index 25a81e5ec6..e206e67756 100644 --- a/src/backend/access/common/toast_internals.c +++ b/src/backend/access/common/toast_internals.c @@ -328,7 +328,7 @@ toast_save_datum(Relation rel, Datum value, toastrel, toastidxs[i]->rd_index->indisunique ? UNIQUE_CHECK_YES : UNIQUE_CHECK_NO, - NULL); + false, NULL); } /* diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index 77433dc8a4..7fa880fc6c 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -488,6 +488,7 @@ bool gininsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { GinState *ginstate = (GinState *) indexInfo->ii_AmCache; diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 3f2b416ce1..0f7a15ec0b 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -156,6 +156,7 @@ bool gistinsert(Relation r, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { GISTSTATE *giststate = (GISTSTATE *) indexInfo->ii_AmCache; diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 7c9ccf446c..7da8b892f4 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -247,6 +247,7 @@ bool hashinsert(Relation rel, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { Datum index_values[1]; diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 3eea215b85..c6f438de72 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1956,6 +1956,7 @@ heapam_index_validate_scan(Relation heapRelation, heapRelation, indexInfo->ii_Unique ? UNIQUE_CHECK_YES : UNIQUE_CHECK_NO, + false, indexInfo); state->tups_inserted += 1; diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 3fb8688f8f..9ad1a18a31 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -179,6 +179,7 @@ index_insert(Relation indexRelation, ItemPointer heap_t_ctid, Relation heapRelation, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { RELATION_CHECKS; @@ -191,7 +192,8 @@ index_insert(Relation indexRelation, return indexRelation->rd_indam->aminsert(indexRelation, values, isnull, heap_t_ctid, heapRelation, - checkUnique, indexInfo); + checkUnique, indexUnchanged, + indexInfo); } /* diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 0abec10798..a3d757c28f 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -199,6 +199,7 @@ bool btinsert(Relation rel, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { bool result; diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c index e4508a2b92..dcffb76e16 100644 --- a/src/backend/access/spgist/spginsert.c +++ b/src/backend/access/spgist/spginsert.c @@ -207,6 +207,7 @@ bool spginsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { SpGistState spgstate; diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c index 538f6a06b8..3e577dd183 100644 --- a/src/backend/catalog/indexing.c +++ b/src/backend/catalog/indexing.c @@ -162,6 +162,7 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple) heapRelation, index->rd_index->indisunique ? UNIQUE_CHECK_YES : UNIQUE_CHECK_NO, + false, indexInfo); } diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c index fc19307bf2..79bd12b7de 100644 --- a/src/backend/commands/constraint.c +++ b/src/backend/commands/constraint.c @@ -175,7 +175,7 @@ unique_key_recheck(PG_FUNCTION_ARGS) */ index_insert(indexRel, values, isnull, &checktid, trigdata->tg_relation, UNIQUE_CHECK_EXISTING, - indexInfo); + false, indexInfo); } else { diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 1b14e9a6eb..a848edb8fd 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -340,8 +340,8 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo, cstate->cur_lineno = buffer->linenos[i]; recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - buffer->slots[i], estate, false, NULL, - NIL); + buffer->slots[i], estate, false, false, + NULL, NIL); ExecARInsertTriggers(estate, resultRelInfo, slots[i], recheckIndexes, cstate->transition_capture); @@ -1085,6 +1085,7 @@ CopyFrom(CopyFromState cstate) myslot, estate, false, + false, NULL, NIL); } diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index c336b238aa..8c663fcda5 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -71,10 +71,8 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; static int MyTriggerDepth = 0; /* - * Note that similar macros also exist in executor/execMain.c. There does not - * appear to be any good header to put them into, given the structures that - * they use, so we let them be duplicated. Be sure to update all if one needs - * to be changed, however. + * The authoritative version of this macro is in executor/execMain.c. Be sure + * to keep everything in sync. */ #define GetAllUpdatedColumns(relinfo, estate) \ (bms_union(exec_rt_fetch((relinfo)->ri_RangeTableIndex, estate)->updatedCols, \ diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index c6b5bcba7b..69bb51b6d0 100644 --- a/src/backend/executor/execIndexing.c +++ b/src/backend/executor/execIndexing.c @@ -124,6 +124,15 @@ typedef enum CEOUC_LIVELOCK_PREVENTING_WAIT } CEOUC_WAIT_MODE; +/* + * The authoritative version of these macro are in executor/execMain.c. Be + * sure to keep everything in sync. + */ +#define GetUpdatedColumns(relinfo, estate) \ + (exec_rt_fetch((relinfo)->ri_RangeTableIndex, estate)->updatedCols) +#define GetExtraUpdatedColumns(relinfo, estate) \ + (exec_rt_fetch((relinfo)->ri_RangeTableIndex, estate)->extraUpdatedCols) + static bool check_exclusion_or_unique_constraint(Relation heap, Relation index, IndexInfo *indexInfo, ItemPointer tupleid, @@ -136,6 +145,11 @@ static bool check_exclusion_or_unique_constraint(Relation heap, Relation index, static bool index_recheck_constraint(Relation index, Oid *constr_procs, Datum *existing_values, bool *existing_isnull, Datum *new_values); +static bool index_unchanged_by_update(ResultRelInfo *resultRelInfo, + EState *estate, IndexInfo *indexInfo, + Relation indexRelation); +static bool index_unchanged_by_update_var_walker(Node *node, + Bitmapset *allUpdatedCols); /* ---------------------------------------------------------------- * ExecOpenIndices @@ -254,6 +268,16 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo) * into all the relations indexing the result relation * when a heap tuple is inserted into the result relation. * + * When 'update' is true, executor is performing an UPDATE + * that could not use an optimization like heapam's HOT (in + * more general terms a call to table_tuple_update() took + * place and set 'update_indexes' to true). Receiving this + * hint makes us consider if we should pass down the + * 'indexUnchanged' hint in turn. That's something that we + * figure out for each index_insert() call iff 'update' is + * true. (When 'update' is false we already know not to pass + * the hint to any index.) + * * Unique and exclusion constraints are enforced at the same * time. This returns a list of index OIDs for any unique or * exclusion constraints that are deferred and that had @@ -263,16 +287,13 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo) * * If 'arbiterIndexes' is nonempty, noDupErr applies only to * those indexes. NIL means noDupErr applies to all indexes. - * - * CAUTION: this must not be called for a HOT update. - * We can't defend against that here for lack of info. - * Should we change the API to make it safer? * ---------------------------------------------------------------- */ List * ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, + bool update, bool noDupErr, bool *specConflict, List *arbiterIndexes) @@ -319,6 +340,7 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, IndexInfo *indexInfo; bool applyNoDupErr; IndexUniqueCheck checkUnique; + bool indexUnchanged; bool satisfiesConstraint; if (indexRelation == NULL) @@ -389,6 +411,16 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, else checkUnique = UNIQUE_CHECK_PARTIAL; + /* + * There's definitely going to be an index_insert() call for this + * index. If we're being called as part of an UPDATE statement, + * consider if the 'indexUnchanged' = true hint should be passed. + */ + indexUnchanged = update && index_unchanged_by_update(resultRelInfo, + estate, + indexInfo, + indexRelation); + satisfiesConstraint = index_insert(indexRelation, /* index relation */ values, /* array of index Datums */ @@ -396,6 +428,7 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, tupleid, /* tid of heap tuple */ heapRelation, /* heap relation */ checkUnique, /* type of uniqueness check to do */ + indexUnchanged, /* UPDATE without logical change? */ indexInfo); /* index AM may need this */ /* @@ -899,3 +932,122 @@ index_recheck_constraint(Relation index, Oid *constr_procs, return true; } + +/* + * Check if ExecInsertIndexTuples() should pass indexUnchanged hint. + * + * When the executor performs an UPDATE that requires a new round of index + * tuples, determine if we should pass 'indexUnchanged' = true hint for one + * single index. + */ +static bool +index_unchanged_by_update(ResultRelInfo *resultRelInfo, EState *estate, + IndexInfo *indexInfo, Relation indexRelation) +{ + Bitmapset *updatedCols = GetUpdatedColumns(resultRelInfo, estate); + Bitmapset *extraUpdatedCols = GetExtraUpdatedColumns(resultRelInfo, estate); + Bitmapset *allUpdatedCols; + bool hasexpression = false; + List *idxExprs; + + /* + * Check for indexed attribute overlap with updated columns. + * + * Only do this for key columns. A change to a non-key column within an + * INCLUDE index should not be considered because that's just payload to + * the index (they're not unlike table TIDs to the index AM). + */ + for (int attr = 0; attr < indexInfo->ii_NumIndexKeyAttrs; attr++) + { + int keycol = indexInfo->ii_IndexAttrNumbers[attr]; + + if (keycol <= 0) + { + /* + * Skip expressions for now, but remember to deal with them later + * on + */ + hasexpression = true; + continue; + } + + if (bms_is_member(keycol - FirstLowInvalidHeapAttributeNumber, + updatedCols) || + bms_is_member(keycol - FirstLowInvalidHeapAttributeNumber, + extraUpdatedCols)) + { + /* Changed key column -- don't hint for this index */ + return false; + } + } + + /* + * When we get this far and index has no expressions, return true so that + * index_insert() call will go on to pass 'indexUnchanged' = true hint. + * + * The _absence_ of an indexed key attribute that overlaps with updated + * attributes (in addition to the total absence of indexed expressions) + * shows that the index as a whole is logically unchanged by UPDATE. + */ + if (!hasexpression) + return true; + + /* + * Need to pass only one bms to expression_tree_walker helper function. + * Avoid allocating memory in common case where there are no extra cols. + */ + if (!extraUpdatedCols) + allUpdatedCols = updatedCols; + else + allUpdatedCols = bms_union(updatedCols, extraUpdatedCols); + + /* + * We have to work slightly harder in the event of indexed expressions, + * but the principle is the same as before: try to find columns (Vars, + * actually) that overlap with known-updated columns. + * + * If we find any matching Vars, don't pass hint for index. Otherwise + * pass hint. + */ + idxExprs = RelationGetIndexExpressions(indexRelation); + hasexpression = index_unchanged_by_update_var_walker((Node *) idxExprs, + allUpdatedCols); + list_free(idxExprs); + if (extraUpdatedCols) + bms_free(allUpdatedCols); + + if (hasexpression) + return false; + + return true; +} + +/* + * Indexed expression helper for index_unchanged_by_update(). + * + * Returns true when Var that appears within allUpdatedCols located. + */ +static bool +index_unchanged_by_update_var_walker(Node *node, Bitmapset *allUpdatedCols) +{ + if (node == NULL) + return false; + + if (IsA(node, Var)) + { + Var *var = (Var *) node; + + if (bms_is_member(var->varattno - FirstLowInvalidHeapAttributeNumber, + allUpdatedCols)) + { + /* Var was updated -- indicates that we should not hint */ + return true; + } + + /* Still haven't found a reason to not pass the hint */ + return false; + } + + return expression_tree_walker(node, index_unchanged_by_update_var_walker, + (void *) allUpdatedCols); +} diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 7179f589f9..31cbc27fa1 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -101,10 +101,10 @@ static char *ExecBuildSlotValueDescription(Oid reloid, static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree); /* - * Note that GetAllUpdatedColumns() also exists in commands/trigger.c. There does - * not appear to be any good header to put it into, given the structures that - * it uses, so we let them be duplicated. Be sure to update both if one needs - * to be changed, however. + * Note that variants of these macros exists in commands/trigger.c and in + * execIndexing.c. There does not appear to be any good header to put it + * into, given the structures that it uses, so we let them be duplicated. Be + * sure to keep everything in sync. */ #define GetInsertedColumns(relinfo, estate) \ (exec_rt_fetch((relinfo)->ri_RangeTableIndex, estate)->insertedCols) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 01d26881e7..1be26f84d9 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -444,8 +444,8 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, NULL, - NIL); + slot, estate, false, false, + NULL, NIL); /* AFTER ROW INSERT Triggers */ ExecARInsertTriggers(estate, resultRelInfo, slot, @@ -512,8 +512,8 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_NumIndices > 0 && update_indexes) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, NULL, - NIL); + slot, estate, true, false, + NULL, NIL); /* AFTER ROW UPDATE Triggers */ ExecARUpdateTriggers(estate, resultRelInfo, diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ab3d655e60..af5659f97f 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -599,7 +599,7 @@ ExecInsert(ModifyTableState *mtstate, /* insert index entries for tuple */ recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, true, + slot, estate, false, true, &specConflict, arbiterIndexes); @@ -640,7 +640,7 @@ ExecInsert(ModifyTableState *mtstate, if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, slot, estate, false, - NULL, NIL); + false, NULL, NIL); } } @@ -1511,7 +1511,7 @@ lreplace:; /* insert index entries for tuple if necessary */ if (resultRelInfo->ri_NumIndices > 0 && update_indexes) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, + slot, estate, true, false, NULL, NIL); } diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 3874939380..f24f06dd9c 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -1309,7 +1309,8 @@ apply_handle_update(StringInfo s) InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0); /* - * Populate updatedCols so that per-column triggers can fire. This could + * Populate updatedCols so that per-column triggers can fire, and so + * executor can correctly pass down indexUnchanged hint. This could * include more columns than were actually changed on the publisher * because the logical replication protocol doesn't contain that * information. But it would for example exclude columns that only exist diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c index 6d3fd5c432..ce4e7486e9 100644 --- a/contrib/bloom/blinsert.c +++ b/contrib/bloom/blinsert.c @@ -198,6 +198,7 @@ bool blinsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { BloomState blstate; diff --git a/contrib/bloom/bloom.h b/contrib/bloom/bloom.h index 23aa7ac441..59ae83d7aa 100644 --- a/contrib/bloom/bloom.h +++ b/contrib/bloom/bloom.h @@ -192,6 +192,7 @@ extern bool blvalidate(Oid opclassoid); extern bool blinsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, struct IndexInfo *indexInfo); extern IndexScanDesc blbeginscan(Relation r, int nkeys, int norderbys); extern int64 blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm); diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index f00268d5b5..ec5741df6d 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -293,6 +293,7 @@ aminsert (Relation indexRelation, ItemPointer heap_tid, Relation heapRelation, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo); Insert a new tuple into an existing index. The values and @@ -308,6 +309,20 @@ aminsert (Relation indexRelation, look into the heap to verify tuple liveness). + + The indexUnchanged boolean value gives a hint + about the nature of the tuple to be indexed. When it is true, + the tuple is a duplicate of some existing tuple in the index. The + new tuple is a logically unchanged successor MVCC tuple version. This + happens when an UPDATE takes place that does not + modify any columns covered by the index, but nevertheless requires a + new version in the index. The index AM may use this hint to decide + to apply bottom-up index deletion in parts of the index where many + versions of the same logical row accumulate. Note that updating a + non-key column does not affect the value of + indexUnchanged. + + The function's Boolean result value is significant only when checkUnique is UNIQUE_CHECK_PARTIAL. diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c b/src/test/modules/dummy_index_am/dummy_index_am.c index 8f4cdab1b3..bcd0b48bf0 100644 --- a/src/test/modules/dummy_index_am/dummy_index_am.c +++ b/src/test/modules/dummy_index_am/dummy_index_am.c @@ -168,6 +168,7 @@ static bool diinsert(Relation index, Datum *values, bool *isnull, ItemPointer ht_ctid, Relation heapRel, IndexUniqueCheck checkUnique, + bool indexUnchanged, IndexInfo *indexInfo) { /* nothing to do */ -- 2.27.0