Hello Fabrízio, hackers, On 2026-Feb-11, Fabrízio de Royes Mello wrote:
> On Wed, Feb 11, 2026 at 4:07 PM Álvaro Herrera <[email protected]> wrote: > > > 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. > > Much better. LGTM! Thanks for looking! However, I had second thoughts about this formulation. Mainly, the fact that one of the output arguments is part of the macro is kinda icky. So I decided that a simpler, less innovative option is to just use a bits32 argument to carry the input booleans, and let the output boolean be a separate argument (which I also moved to appear last in the argument list, as we normally do). This also means the list of indexes continues to be its own argument. But, as I said, this is less innovative, which I think is mostly good. And it definitely reads better than currently. There might be places in executor.h to reuse the f831d4accda0 thingy, but this is probably not it. Thanks, -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Los dioses no protegen a los insensatos. Éstos reciben protección de otros insensatos mejor dotados" (Luis Wu, Mundo Anillo)
>From f98ee25bd47f1e03ce421032198819f3d94103ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Mon, 16 Feb 2026 18:49:53 +0100 Subject: [PATCH] Use bitmask for ExecInsertIndexTuples options ... instead of passing a bunch of separate booleans. --- src/backend/commands/copyfrom.c | 11 ++++----- src/backend/executor/execIndexing.c | 33 +++++++++++++------------- src/backend/executor/execReplication.c | 24 ++++++++++++------- src/backend/executor/nodeModifyTable.c | 20 +++++++++------- src/include/executor/executor.h | 11 +++++---- 5 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 25ee20b23db..ecb78b8adb9 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, NIL, 0, + NULL); ExecARInsertTriggers(estate, resultRelInfo, slots[i], recheckIndexes, cstate->transition_capture); @@ -1431,11 +1431,8 @@ CopyFrom(CopyFromState cstate) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, myslot, estate, - false, - false, - NULL, - NIL, - false); + NIL, 0, + NULL); } /* AFTER ROW INSERT Triggers */ diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index f0ba7eac87d..5fb79159930 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 EIIT_IS_UPDATE is set and EIIT_ONLY_SUMMARIZING isn't, * 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 EIIT_IS_UPDATE is set. + * (When that flag is not set we already know not to pass the * hint to any index.) * - * If onlySummarizing is set, an equivalent optimization to + * If EIIT_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 EIIT_NO_DUPE_ERROR, * the same is done for non-deferred constraints, but report * if conflict was speculative or deferred conflict to caller) * - * If 'arbiterIndexes' is nonempty, noDupErr applies only to - * those indexes. NIL means noDupErr applies to all indexes. + * If 'arbiterIndexes' is nonempty, EIIT_NO_DUPE_ERROR applies only to + * those indexes. NIL means EIIT_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) + bits32 flags, + bool *specConflict) { 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 ((flags & EIIT_ONLY_SUMMARIZING) && !indexInfo->ii_Summarizing) continue; /* Check for partial index */ @@ -409,7 +407,7 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, isnull); /* Check whether to apply noDupErr to this index */ - applyNoDupErr = noDupErr && + applyNoDupErr = (flags & EIIT_NO_DUPE_ERROR) && (arbiterIndexes == NIL || list_member_oid(arbiterIndexes, 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 = ((flags & EIIT_IS_UPDATE) && + index_unchanged_by_update(resultRelInfo, + estate, + indexInfo, + indexRelation)); satisfiesConstraint = index_insert(indexRelation, /* index relation */ diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 743b1ee2b28..cf22775871d 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -846,11 +846,14 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo, conflictindexes = resultRelInfo->ri_onConflictArbiterIndexes; if (resultRelInfo->ri_NumIndices > 0) + { + bits32 flags = conflictindexes != NIL ? EIIT_NO_DUPE_ERROR : 0; + recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, - conflictindexes ? true : false, - &conflict, - conflictindexes, false); + slot, estate, + conflictindexes, flags, + &conflict); + } /* * Checks the conflict indexes to fetch the conflicting local row and @@ -943,11 +946,16 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, conflictindexes = resultRelInfo->ri_onConflictArbiterIndexes; if (resultRelInfo->ri_NumIndices > 0 && (update_indexes != TU_None)) + { + bits32 flags = EIIT_IS_UPDATE; + + flags |= conflictindexes != NIL ? EIIT_NO_DUPE_ERROR : 0; + flags |= update_indexes == TU_Summarizing ? EIIT_ONLY_SUMMARIZING : 0; recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, true, - conflictindexes ? true : false, - &conflict, conflictindexes, - (update_indexes == TU_Summarizing)); + slot, estate, + conflictindexes, flags, + &conflict); + } /* * Refer to the comments above the call to CheckAndReportConflict() in diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 6802fc13e95..5dbf866a1b9 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1226,10 +1226,10 @@ ExecInsert(ModifyTableContext *context, /* insert index entries for tuple */ recheckIndexes = ExecInsertIndexTuples(resultRelInfo, - slot, estate, false, true, - &specConflict, + slot, estate, arbiterIndexes, - false); + EIIT_NO_DUPE_ERROR, + &specConflict); /* adjust the tuple's state accordingly */ table_tuple_complete_speculative(resultRelationDesc, slot, @@ -1267,9 +1267,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, NIL, 0, + NULL); } } @@ -2356,11 +2355,14 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, /* insert index entries for tuple if necessary */ if (resultRelInfo->ri_NumIndices > 0 && (updateCxt->updateIndexes != TU_None)) + { + bits32 flags = EIIT_IS_UPDATE; + + flags |= updateCxt->updateIndexes == TU_Summarizing ? EIIT_ONLY_SUMMARIZING : 0; recheckIndexes = ExecInsertIndexTuples(resultRelInfo, slot, context->estate, - true, false, - NULL, NIL, - (updateCxt->updateIndexes == TU_Summarizing)); + NIL, flags, NULL); + } /* AFTER ROW UPDATE Triggers */ ExecARUpdateTriggers(context->estate, resultRelInfo, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 55a7d930d26..e788c5c7aee 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -739,12 +739,15 @@ extern Bitmapset *ExecGetAllUpdatedCols(ResultRelInfo *relinfo, EState *estate); */ extern void ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative); extern void ExecCloseIndices(ResultRelInfo *resultRelInfo); + +/* flags for ExecInsertIndexTuples */ +#define EIIT_IS_UPDATE (1<<0) +#define EIIT_NO_DUPE_ERROR (1<<1) +#define EIIT_ONLY_SUMMARIZING (1<<2) extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, - bool update, - bool noDupErr, - bool *specConflict, List *arbiterIndexes, - bool onlySummarizing); + List *arbiterIndexes, bits32 options, + bool *specConflict); extern bool ExecCheckIndexConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate, ItemPointer conflictTid, -- 2.47.3
