On Tue, Dec 02, 2025 at 05:51:15PM -0500, Tom Lane wrote: > Chao Li <[email protected]> writes: >> On Dec 3, 2025, at 06:00, Nathan Bossart <[email protected]> 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
>From 804bb1097a44cc35178f3e1de46d17422ce7b67f Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> 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)
>From ccdde305aad8277bf91cf02577c723a008919288 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> 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)
