From: Álvaro Herrera <alvherre@kurilemu.de>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: make ExecInsertIndexTuples arguments less bad
Date: Wed, 11 Feb 2026 20:07:39 +0100
Message-ID: <202602111846.xpvuccb3inbx@alvherre.pgsql> (raw)
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)
Attachments:
[text/x-diff] 0001-Split-out-ExecInsertIndexTuples-arguments.patch (12.2K, ../202602111846.xpvuccb3inbx@alvherre.pgsql/2-0001-Split-out-ExecInsertIndexTuples-arguments.patch)
download | inline diff:
From 7732ab00600b5b58b27f2e70b9c7618b690d627c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
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.hindex 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.cindex 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.cindex 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.cindex 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.cindex 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.listindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de, pgsql-hackers@lists.postgresql.org
Subject: Re: make ExecInsertIndexTuples arguments less bad
In-Reply-To: <202602111846.xpvuccb3inbx@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox