Feedback-ID: ie3de48e3:Fastmail
Date: Mon, 16 Feb 2026 19:19:33 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: =?utf-8?B?RmFicsOtemlv?= de Royes Mello <fabriziomello@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: make ExecInsertIndexTuples arguments less bad
Message-ID: <202602161805.jwp2f4s5svmp@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="iqj6mfwrrhuzgzze"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAFcNs+pX-bjb80_g_oMwbdrWa9vT+Ke7n7qCMETXPghSwugbJg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202602161805.jwp2f4s5svmp%40alvherre.pgsql>
Precedence: bulk


--iqj6mfwrrhuzgzze
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

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 <alvherre@kurilemu.de> 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)

--iqj6mfwrrhuzgzze
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Use-bitmask-for-ExecInsertIndexTuples-options.patch"

From f98ee25bd47f1e03ce421032198819f3d94103ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
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


--iqj6mfwrrhuzgzze--