On 03.09.26 15:52, Tom Lane wrote:
Peter Eisentraut <[email protected]> writes:
I sense that nobody is going to have issues with this. Any objections
to backpatching this? I suspect that over time many more people are
going to run into this, so keeping the backbranches warning-free seems
sensible.
Agreed, we usually like to keep the back branches warning-free too.
But I concur with Andreas that 0005 looks a bit half-baked.
Maybe that's the right amount of effort for the back branches,
but something else should happen in HEAD.
Right. Here is another patch at the end (0006) that I propose for
master only that makes using unsupported options an error. Some
Windows-only options already behaved that way, so this just makes the
rest consistent.
From a56270a832812fbd9255c1da5cef1324216308f3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Aug 2026 23:06:46 +0200
Subject: [PATCH v2 1/6] Remove unused global variable [xlog.c]
src/backend/access/transam/xlog.c:675:19: error: variable
'LocalMinRecoveryPointTLI' set but not used [-Werror,-Wunused-but-set-global]
This was apparently never used (initial commit 70e81861fad).
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/backend/access/transam/xlog.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 3203f2fd4ee..b98fe621964 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -672,7 +672,6 @@ static TimeLineID openLogTLI = 0;
* Those values are kept consistent as long as crash recovery runs.
*/
static XLogRecPtr LocalMinRecoveryPoint;
-static TimeLineID LocalMinRecoveryPointTLI;
static bool updateMinRecoveryPoint = true;
/*
@@ -2752,7 +2751,6 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
/* update local copy */
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
if (!XLogRecPtrIsValid(LocalMinRecoveryPoint))
updateMinRecoveryPoint = false;
@@ -2787,7 +2785,6 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
ControlFile->minRecoveryPointTLI =
newMinRecoveryPointTLI;
UpdateControlFile();
LocalMinRecoveryPoint = newMinRecoveryPoint;
- LocalMinRecoveryPointTLI = newMinRecoveryPointTLI;
ereport(DEBUG2,
errmsg_internal("updated min recovery
point to %X/%08X on timeline %u",
@@ -3201,7 +3198,6 @@ XLogNeedsFlush(XLogRecPtr record)
if (!LWLockConditionalAcquire(ControlFileLock, LW_SHARED))
return true;
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
LWLockRelease(ControlFileLock);
/*
@@ -6207,12 +6203,10 @@ StartupXLOG(void)
if (InArchiveRecovery)
{
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI =
ControlFile->minRecoveryPointTLI;
}
else
{
LocalMinRecoveryPoint = InvalidXLogRecPtr;
- LocalMinRecoveryPointTLI = 0;
}
/* Check that the GUCs used to generate the WAL allow recovery
*/
@@ -6749,7 +6743,6 @@ SwitchIntoArchiveRecovery(XLogRecPtr EndRecPtr,
TimeLineID replayTLI)
}
/* update local copy */
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
/*
* The startup process can update its local copy of minRecoveryPoint
from
@@ -8316,7 +8309,6 @@ CreateRestartPoint(int flags)
/* update local copy */
LocalMinRecoveryPoint =
ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI =
ControlFile->minRecoveryPointTLI;
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
@@ -9161,7 +9153,6 @@ xlog_redo(XLogReaderState *record)
if (InArchiveRecovery)
{
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI =
ControlFile->minRecoveryPointTLI;
}
if (XLogRecPtrIsValid(LocalMinRecoveryPoint) &&
LocalMinRecoveryPoint < lsn)
{
@@ -9307,7 +9298,6 @@ xlog2_redo(XLogReaderState *record)
if (InArchiveRecovery)
{
LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
- LocalMinRecoveryPointTLI =
ControlFile->minRecoveryPointTLI;
}
if (XLogRecPtrIsValid(LocalMinRecoveryPoint) &&
LocalMinRecoveryPoint < lsn)
{
--
2.55.0
From 8f8a6b14458125c37c23a9f8a0842249e79ad4d4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Aug 2026 23:08:56 +0200
Subject: [PATCH v2 2/6] Remove unused global variable [test_resowner]
src/test/modules/test_resowner/test_resowner_many.c:49:29: error: variable
'current_release_phase' set but not used [-Werror,-Wunused-but-set-global]
This was apparently never used (initial commit b8bff07daa8).
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/test/modules/test_resowner/test_resowner_many.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/test/modules/test_resowner/test_resowner_many.c
b/src/test/modules/test_resowner/test_resowner_many.c
index 4bb34ca7e4c..949313276a2 100644
--- a/src/test/modules/test_resowner/test_resowner_many.c
+++ b/src/test/modules/test_resowner/test_resowner_many.c
@@ -43,10 +43,9 @@ typedef struct
} ManyTestResource;
/*
- * Current release phase, and priority of last call to the release callback.
+ * Current release priority of last call to the release callback.
* This is used to check that the resources are released in correct order.
*/
-static ResourceReleasePhase current_release_phase;
static uint32 last_release_priority = 0;
/* prototypes for local functions */
@@ -272,18 +271,15 @@ test_resowner_many(PG_FUNCTION_ARGS)
/* Start releasing */
elog(NOTICE, "releasing resources before locks");
- current_release_phase = RESOURCE_RELEASE_BEFORE_LOCKS;
last_release_priority = 0;
ResourceOwnerRelease(resowner, RESOURCE_RELEASE_BEFORE_LOCKS, false,
false);
Assert(GetTotalResourceCount(before_kinds, nkinds) == 0);
elog(NOTICE, "releasing locks");
- current_release_phase = RESOURCE_RELEASE_LOCKS;
last_release_priority = 0;
ResourceOwnerRelease(resowner, RESOURCE_RELEASE_LOCKS, false, false);
elog(NOTICE, "releasing resources after locks");
- current_release_phase = RESOURCE_RELEASE_AFTER_LOCKS;
last_release_priority = 0;
ResourceOwnerRelease(resowner, RESOURCE_RELEASE_AFTER_LOCKS, false,
false);
Assert(GetTotalResourceCount(before_kinds, nkinds) == 0);
--
2.55.0
From 8f3d347423c94040b600e80961d547e3ca0a0190 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Aug 2026 23:10:47 +0200
Subject: [PATCH v2 3/6] Remove unused global variable [pg_dump]
src/bin/pg_dump/pg_dump_sort.c:160:15: error: variable 'preDataBoundId' set but
not used [-Werror,-Wunused-but-set-global]
This was apparently never used (initial commit a1ef01fe163).
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/bin/pg_dump/pg_dump_sort.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 5b3ea0f2a18..4f3469d4396 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -157,7 +157,6 @@ static const int dbObjectTypePriority[] =
StaticAssertDecl(lengthof(dbObjectTypePriority) == NUM_DUMPABLE_OBJECT_TYPES,
"array length mismatch");
-static DumpId preDataBoundId;
static DumpId postDataBoundId;
@@ -569,7 +568,6 @@ sortDumpableObjects(DumpableObject **objs, int numObjs,
* Saving the boundary IDs in static variables is a bit grotty, but
seems
* better than adding them to parameter lists of subsidiary functions.
*/
- preDataBoundId = preBoundaryId;
postDataBoundId = postBoundaryId;
ordering = pg_malloc_array(DumpableObject *, numObjs);
--
2.55.0
From f4a3b3aa747b49b4c4d195d76bc7cef4bdbcb55f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Aug 2026 23:18:31 +0200
Subject: [PATCH v2 4/6] Remove unused global variable [postmaster]
src/backend/postmaster/postmaster.c:372:13: error: variable
'ReachedNormalRunning' set but not used [-Werror,-Wunused-but-set-global]
The last use was removed in commit 39969e2a1e4.
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/backend/postmaster/postmaster.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 5f7a7b368e1..ef300a6c45a 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -369,8 +369,6 @@ static time_t AbortStartTime = 0;
/* Length of said timeout */
#define SIGKILL_CHILDREN_AFTER_SECS 5
-static bool ReachedNormalRunning = false; /* T if we've reached PM_RUN */
-
bool ClientAuthInProgress = false; /* T during new-client
* authentication */
@@ -2362,7 +2360,6 @@ process_pm_child_exit(void)
StartupStatus = STARTUP_NOT_RUNNING;
FatalError = false;
AbortStartTime = 0;
- ReachedNormalRunning = true;
UpdatePMState(PM_RUN);
connsAllowed = true;
--
2.55.0
From aba52216c8bcf30bc271c0bdc79c8147c4eb412b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Aug 2026 23:23:12 +0200
Subject: [PATCH v2 5/6] Silence warnings about unused global variables
[pg_ctl]
Several global variables are only used in Windows build.
This patch marks these with an unused attribute to silence warnings.
Alternatively, this code should perhaps be restricted to Windows more
consistently. For example, currently you can use the -e option on any
platform, but it only does something on Windows.
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/bin/pg_ctl/pg_ctl.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 6c604e2d962..714e314a9e4 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -75,7 +75,7 @@ StaticAssertDecl(USECS_PER_SEC % WAITS_PER_SEC == 0,
static bool do_wait = true;
static int wait_seconds = DEFAULT_WAIT;
-static bool wait_seconds_arg = false;
+pg_attribute_unused() static bool wait_seconds_arg = false;
static bool silent_mode = false;
static ShutdownMode shutdown_mode = FAST_MODE;
static int sig = SIGINT; /* default */
@@ -87,10 +87,10 @@ static char *post_opts = NULL;
static const char *progname;
static char *log_file = NULL;
static char *exec_path = NULL;
-static char *event_source = NULL;
-static char *register_servicename = "PostgreSQL"; /* FIXME: + version ID?
*/
-static char *register_username = NULL;
-static char *register_password = NULL;
+pg_attribute_unused() static char *event_source = NULL;
+pg_attribute_unused() static char *register_servicename = "PostgreSQL"; /*
FIXME: + version ID? */
+pg_attribute_unused() static char *register_username = NULL;
+pg_attribute_unused() static char *register_password = NULL;
static char *argv0 = NULL;
static bool allow_core_files = false;
static time_t start_time;
--
2.55.0
From a458e130f263372e430f45cf583f9018afd21ee2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 4 Sep 2026 20:01:51 +0200
Subject: [PATCH v2 6/6] pg_ctl: Error out on Windows-only options elsewhere
Several options are used only on Windows, but pg_ctl accepted them
anyway on all platforms but didn't do anything with them. This
changes it to report an error instead if the platform doesn't make use
of them. The affected options are -e, -N, -P, -U. The option -S
already errored out on non-Windows platforms; the new behavior of the
other options is aligned with that.
The --help output already didn't list these options on non-Windows
builds, so the fact that they were accepted silently was already
inconsistent. The man page already lists them as "Options for
Windows".
Several global variables that were set but then only read on Windows,
are now in #ifdef WIN32 blocks.
Discussion:
https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
---
src/bin/pg_ctl/pg_ctl.c | 44 ++++++++++++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 714e314a9e4..199f6c55b4a 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -75,7 +75,9 @@ StaticAssertDecl(USECS_PER_SEC % WAITS_PER_SEC == 0,
static bool do_wait = true;
static int wait_seconds = DEFAULT_WAIT;
-pg_attribute_unused() static bool wait_seconds_arg = false;
+#ifdef WIN32
+static bool wait_seconds_arg = false;
+#endif
static bool silent_mode = false;
static ShutdownMode shutdown_mode = FAST_MODE;
static int sig = SIGINT; /* default */
@@ -87,10 +89,12 @@ static char *post_opts = NULL;
static const char *progname;
static char *log_file = NULL;
static char *exec_path = NULL;
-pg_attribute_unused() static char *event_source = NULL;
-pg_attribute_unused() static char *register_servicename = "PostgreSQL"; /*
FIXME: + version ID? */
-pg_attribute_unused() static char *register_username = NULL;
-pg_attribute_unused() static char *register_password = NULL;
+#ifdef WIN32
+static char *event_source = NULL;
+static char *register_servicename = "PostgreSQL"; /* FIXME: + version ID?
*/
+static char *register_username = NULL;
+static char *register_password = NULL;
+#endif
static char *argv0 = NULL;
static bool allow_core_files = false;
static time_t start_time;
@@ -2292,7 +2296,13 @@ main(int argc, char **argv)
break;
}
case 'e':
+#ifdef WIN32
event_source = pg_strdup(optarg);
+#else
+ write_stderr(_("%s: -%c option not supported on
this platform\n"),
+ progname, c);
+ exit(1);
+#endif
break;
case 'l':
log_file = pg_strdup(optarg);
@@ -2301,7 +2311,13 @@ main(int argc, char **argv)
set_mode(optarg);
break;
case 'N':
+#ifdef WIN32
register_servicename = pg_strdup(optarg);
+#else
+ write_stderr(_("%s: -%c option not supported on
this platform\n"),
+ progname, c);
+ exit(1);
+#endif
break;
case 'o':
/* append option? */
@@ -2319,7 +2335,13 @@ main(int argc, char **argv)
exec_path = pg_strdup(optarg);
break;
case 'P':
+#ifdef WIN32
register_password = pg_strdup(optarg);
+#else
+ write_stderr(_("%s: -%c option not supported on
this platform\n"),
+ progname, c);
+ exit(1);
+#endif
break;
case 's':
silent_mode = true;
@@ -2328,21 +2350,29 @@ main(int argc, char **argv)
#ifdef WIN32
set_starttype(optarg);
#else
- write_stderr(_("%s: -S option not supported on
this platform\n"),
- progname);
+ write_stderr(_("%s: -%c option not supported on
this platform\n"),
+ progname, c);
exit(1);
#endif
break;
case 't':
wait_seconds = atoi(optarg);
+#ifdef WIN32
wait_seconds_arg = true;
+#endif
break;
case 'U':
+#ifdef WIN32
if (strchr(optarg, '\\'))
register_username = pg_strdup(optarg);
else
/* Prepend .\ for local accounts */
register_username = psprintf(".\\%s",
optarg);
+#else
+ write_stderr(_("%s: -%c option not supported on
this platform\n"),
+ progname, c);
+ exit(1);
+#endif
break;
case 'w':
do_wait = true;
--
2.55.0
