Date: Tue, 5 May 2026 16:47:07 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Chao Li <li.evan.chao@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgindent versus struct members and typedefs
Message-ID: <afplW8DvRONDc1yl@nathan>
References: <aS9hkwnkWf3dZIA_@nathan>
 <347EA055-D626-414C-B10D-7E70041B9B55@gmail.com>
 <509793.1764715875@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="k2Vi/uo5iFea1Gni"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <509793.1764715875@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/afplW8DvRONDc1yl%40nathan>
Precedence: bulk


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

On Tue, Dec 02, 2025 at 05:51:15PM -0500, Tom Lane wrote:
> Chao Li <li.evan.chao@gmail.com> writes:
>> On Dec 3, 2025, at 06:00, Nathan Bossart <nathandbossart@gmail.com> wrote:
>>> I tried to fix pgindent for a few, but the code is basically impenetrable.
>>> I didn't find any fixes upstream [0], either.  As noted above, we could
>>> also fix it by avoiding the naming conflicts.  However, I can't imagine
>>> that's worth the churn, and I've already spent way too much time on this,
>>> so IMHO the best thing to do here is nothing.
> 
>> I think that’s fine.
> 
> Agreed, not worth the trouble to fool with.

For fun, I spent some time with an AI tool to develop the attached fix for
this problem.  The explanation seems reasonable to me, although I am by no
means a pgindent expert.  When I looked at this in December, I did find
this similar commit from upstream [0], but I failed to make the connection
with last_u_d.  0002 is the result of a pgindent run after applying 0001.
You'll notice that it fixes the exact set of cases I found with grep
upthread.

[0] https://github.com/pstef/freebsd_indent/commit/afa2239

-- 
nathan

--k2Vi/uo5iFea1Gni
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0001-pgindent-Fix-spacing-after-when-member-name-match.patch

From 804bb1097a44cc35178f3e1de46d17422ce7b67f Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 5 May 2026 16:03:13 -0500
Subject: [PATCH v1 1/2] pgindent: Fix spacing after != when member name
 matches typedef.

When a struct member name matches a registered typedef, pgindent
removes the space after "!=" (and some other operators):

    entry->dsh.dsa_handle !=DSA_HANDLE_INVALID

In lexi.c, when an identifier matches the typedef list, the code
sets last_u_d = true before jumping to found_typename.  This flag
tells the lexer that the next operator should be treated as unary,
which is correct for type names (e.g., "Datum *x" needs * to be
unary, not multiplication).

However, found_typename has a guard that recognizes when a typedef
name appears after "." or "->": in that case, the name is a struct
member, not a type, so it's returned as a plain identifier.  The
problem is that last_u_d has already been set to true and is never
corrected.  This causes the next operator (e.g., "!=") to be
misclassified as unary, which suppresses the space after it.

To fix, only set last_u_d when the token won't be caught by the
guard, i.e., when it will actually be treated as a type name.

This also explains why "==" was not affected: the "==" case always
returns binary_op regardless of last_u_d, while "!=" checks
last_u_d to decide.
---
 src/tools/pg_bsd_indent/lexi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/tools/pg_bsd_indent/lexi.c b/src/tools/pg_bsd_indent/lexi.c
index 943bf7ce6b0..e846188d6f4 100644
--- a/src/tools/pg_bsd_indent/lexi.c
+++ b/src/tools/pg_bsd_indent/lexi.c
@@ -363,7 +363,8 @@ lexi(struct parser_state *state)
 		  bsearch(s_token, typenames, typename_top + 1,
 		    sizeof(typenames[0]), strcmp_type))) {
 		state->keyword = 4;	/* a type name */
-		state->last_u_d = true;
+		if (state->last_token != period && state->last_token != unary_op)
+		    state->last_u_d = true;
 	        goto found_typename;
 	    }
 	} else {			/* we have a keyword */
-- 
2.50.1 (Apple Git-155)


--k2Vi/uo5iFea1Gni
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=v1-0002-run-pgindent.patch

From ccdde305aad8277bf91cf02577c723a008919288 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 5 May 2026 16:04:14 -0500
Subject: [PATCH v1 2/2] run pgindent

---
 src/backend/replication/logical/logicalfuncs.c | 2 +-
 src/backend/storage/ipc/dsm_registry.c         | 2 +-
 src/bin/pg_basebackup/pg_basebackup.c          | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 512013b0ef0..71fbaf72269 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -218,7 +218,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 		 * what we need.
 		 */
 		if (!binary &&
-			ctx->options.output_type !=OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
+			ctx->options.output_type != OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("logical decoding output plugin \"%s\" produces binary output, but function \"%s\" expects textual data",
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index 2b56977659b..b9961c26019 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -479,7 +479,7 @@ pg_get_dsm_registry_allocations(PG_FUNCTION_ARGS)
 				 entry->dsa.handle != DSA_HANDLE_INVALID)
 			vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsa.handle));
 		else if (entry->type == DSMR_ENTRY_TYPE_DSH &&
-				 entry->dsh.dsa_handle !=DSA_HANDLE_INVALID)
+				 entry->dsh.dsa_handle != DSA_HANDLE_INVALID)
 			vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsh.dsa_handle));
 		else
 			nulls[2] = true;
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index c1a4672aa6f..80dc3bbc8da 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1282,7 +1282,7 @@ ReceiveArchiveStream(PGconn *conn, pg_compress_specification *compress)
 	ReceiveCopyData(conn, ReceiveArchiveStreamChunk, &state);
 
 	/* If we wrote the backup manifest to a file, close the file. */
-	if (state.manifest_file !=NULL)
+	if (state.manifest_file != NULL)
 	{
 		fclose(state.manifest_file);
 		state.manifest_file = NULL;
@@ -1341,7 +1341,7 @@ ReceiveArchiveStreamChunk(size_t r, char *copybuf, void *callback_data)
 
 				/* Sanity check. */
 				if (state->manifest_buffer != NULL ||
-					state->manifest_file !=NULL)
+					state->manifest_file != NULL)
 					pg_fatal("archives must precede manifest");
 
 				/* Parse the rest of the CopyData message. */
@@ -1406,7 +1406,7 @@ ReceiveArchiveStreamChunk(size_t r, char *copybuf, void *callback_data)
 					appendPQExpBuffer(state->manifest_buffer, copybuf + 1,
 									  r - 1);
 				}
-				else if (state->manifest_file !=NULL)
+				else if (state->manifest_file != NULL)
 				{
 					/* Manifest data, write to disk. */
 					if (fwrite(copybuf + 1, r - 1, 1,
-- 
2.50.1 (Apple Git-155)


--k2Vi/uo5iFea1Gni--