Hello, The arguments to ExecInsertIndexTuples() are rather unhelpful to read; patching them is messy and hard to follow. How about we reuse the pattern we used in commit f831d4accda0 to make them less bad? I think the code is much nicer to read this way.
-- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint)
>From 7732ab00600b5b58b27f2e70b9c7618b690d627c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Wed, 11 Feb 2026 17:35:34 +0100 Subject: [PATCH] Split out ExecInsertIndexTuples() arguments The way we call this function is somewhat messy, and further patches make it messier. We can simplify the way it looks by defining a struct that carries some of its arguments. This is modelled after the change to ArchiveEntry() in commit f831d4accda0. --- src/include/executor/executor.h | 16 ++++++-- src/backend/executor/execIndexing.c | 53 +++++++++++++------------- src/backend/executor/nodeModifyTable.c | 19 +++++---- src/backend/executor/execReplication.c | 18 +++++---- src/backend/commands/copyfrom.c | 10 ++--- src/tools/pgindent/typedefs.list | 1 + 6 files changed, 61 insertions(+), 56 deletions(-) diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 55a7d930d26..e02c88b422d 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -739,12 +739,20 @@ extern Bitmapset *ExecGetAllUpdatedCols(ResultRelInfo *relinfo, EState *estate); */ extern void ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative); extern void ExecCloseIndices(ResultRelInfo *resultRelInfo); + +/* Options for ExecInsertIndexTuples */ +typedef struct EIITOptions +{ + bool is_update; /* is it an update? */ + bool no_dupe_error; /* no throw in case of duplicates */ + bool only_summarizing; /* only affect summarizing indexes */ + List *arbiter_indexes; /* indexes to check for dupes; NIL=all */ + bool *spec_conflict; /* output: was a conflict detected? */ +} EIITOptions; +#define EIIT_OPTS(...) &(EIITOptions){__VA_ARGS__} extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, - bool update, - bool noDupErr, - bool *specConflict, List *arbiterIndexes, - bool onlySummarizing); + EIITOptions *options); extern bool ExecCheckIndexConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, ItemPointer conflictTid, diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index 6ae0f959592..1f0c4c3f61a 100644 --- a/src/backend/executor/execIndexing.c +++ b/src/backend/executor/execIndexing.c @@ -276,18 +276,18 @@ 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 and 'onlySummarizing' is false, + * When options->is_update is true and options->only_summarizing is false, * 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 TU_All). 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 + * each index_insert() call iff options->is_update is true. + * (When options->is_update is false we already know not to pass the * hint to any index.) * - * If onlySummarizing is set, an equivalent optimization to + * If options->only_summarizing is set, an equivalent optimization to * HOT has been applied and any updated columns are indexed * only by summarizing indexes (or in more general terms a * call to table_tuple_update() took place and set @@ -298,23 +298,21 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo) * 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 - * potential (unconfirmed) conflicts. (if noDupErr == true, + * potential (unconfirmed) conflicts. (If options->no_dupe_error is true, * the same is done for non-deferred constraints, but report - * if conflict was speculative or deferred conflict to caller) + * if conflict was speculative or deferred conflict to caller + * by setting options->spec_conflict). * - * If 'arbiterIndexes' is nonempty, noDupErr applies only to - * those indexes. NIL means noDupErr applies to all indexes. + * If options->arbiter_indexes is nonempty, options->no_dupe_error + * applies only to those indexes. NIL means no_dupe_error applies to + * all indexes. * ---------------------------------------------------------------- */ List * ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, - bool update, - bool noDupErr, - bool *specConflict, - List *arbiterIndexes, - bool onlySummarizing) + EIITOptions *options) { ItemPointer tupleid = &slot->tts_tid; List *result = NIL; @@ -374,7 +372,7 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, * Skip processing of non-summarizing indexes if we only update * summarizing indexes */ - if (onlySummarizing && !indexInfo->ii_Summarizing) + if (options->only_summarizing && !indexInfo->ii_Summarizing) continue; /* Check for partial index */ @@ -408,10 +406,10 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, values, isnull); - /* Check whether to apply noDupErr to this index */ - applyNoDupErr = noDupErr && - (arbiterIndexes == NIL || - list_member_oid(arbiterIndexes, + /* Check whether to apply no_dupe_error to this index */ + applyNoDupErr = options->no_dupe_error && + (options->arbiter_indexes == NIL || + list_member_oid(options->arbiter_indexes, indexRelation->rd_index->indexrelid)); /* @@ -441,10 +439,11 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, * 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); + indexUnchanged = options->is_update && + index_unchanged_by_update(resultRelInfo, + estate, + indexInfo, + indexRelation); satisfiesConstraint = index_insert(indexRelation, /* index relation */ @@ -462,9 +461,9 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, * always insert first and then check. If the constraint is deferred, * we check now anyway, but don't throw error on violation or wait for * a conclusive outcome from a concurrent insertion; instead we'll - * queue a recheck event. Similarly, noDupErr callers (speculative - * inserters) will recheck later, and wait for a conclusive outcome - * then. + * queue a recheck event. Similarly, no_dupe_error callers + * (speculative inserters) will recheck later, and wait for a + * conclusive outcome then. * * An index for an exclusion constraint can't also be UNIQUE (not an * essential property, we just don't allow it in the grammar), so no @@ -510,8 +509,8 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, * speculative conflict, since that always requires a restart. */ result = lappend_oid(result, RelationGetRelid(indexRelation)); - if (indexRelation->rd_index->indimmediate && specConflict) - *specConflict = true; + if (indexRelation->rd_index->indimmediate && options->spec_conflict) + *options->spec_conflict = true; } } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index f5e9d369940..b0be0e8fe98 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1197,10 +1197,10 @@ ExecInsert(ModifyTableContext *context, /* insert index entries for tuple */ recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, true, - &specConflict, - arbiterIndexes, - false); + slot, estate, + EIIT_OPTS(.no_dupe_error = true, + .spec_conflict = &specConflict, + .arbiter_indexes = arbiterIndexes)); /* adjust the tuple's state accordingly */ table_tuple_complete_speculative(resultRelationDesc, slot, @@ -1238,9 +1238,8 @@ ExecInsert(ModifyTableContext *context, /* insert index entries for tuple */ if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, - false, NULL, NIL, - false); + slot, estate, + EIIT_OPTS()); } } @@ -2329,9 +2328,9 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, if (resultRelInfo->ri_NumIndices > 0 && (updateCxt->updateIndexes != TU_None)) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, slot, context->estate, - true, false, - NULL, NIL, - (updateCxt->updateIndexes == TU_Summarizing)); + EIIT_OPTS(.is_update = true, + .only_summarizing = + updateCxt->updateIndexes == TU_Summarizing)); /* AFTER ROW UPDATE Triggers */ ExecARUpdateTriggers(context->estate, resultRelInfo, diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 743b1ee2b28..039cca53705 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -847,10 +847,10 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, - conflictindexes ? true : false, - &conflict, - conflictindexes, false); + slot, estate, + EIIT_OPTS(.no_dupe_error = conflictindexes != NIL, + .spec_conflict = &conflict, + .arbiter_indexes = conflictindexes)); /* * Checks the conflict indexes to fetch the conflicting local row and @@ -944,10 +944,12 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_NumIndices > 0 && (update_indexes != TU_None)) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, true, - conflictindexes ? true : false, - &conflict, conflictindexes, - (update_indexes == TU_Summarizing)); + slot, estate, + EIIT_OPTS(.is_update = true, + .no_dupe_error = conflictindexes != NIL, + .spec_conflict = &conflict, + .arbiter_indexes = conflictindexes, + .only_summarizing = (update_indexes == TU_Summarizing))); /* * Refer to the comments above the call to CheckAndReportConflict() in diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 25ee20b23db..c0597266a82 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -572,8 +572,8 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo, cstate->cur_lineno = buffer->linenos[i]; recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - buffer->slots[i], estate, false, - false, NULL, NIL, false); + buffer->slots[i], estate, + EIIT_OPTS()); ExecARInsertTriggers(estate, resultRelInfo, slots[i], recheckIndexes, cstate->transition_capture); @@ -1431,11 +1431,7 @@ CopyFrom(CopyFromState cstate) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, myslot, estate, - false, - false, - NULL, - NIL, - false); + EIIT_OPTS()); } /* AFTER ROW INSERT Triggers */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 39c76691c86..bd1f6e2fc44 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -695,6 +695,7 @@ DynamicZoneAbbrev ECDerivesEntry ECDerivesKey EDGE +EIITOptions ENGINE EOM_flatten_into_method EOM_get_flat_size_method -- 2.47.3
