add missing PQfinish() calls to vacuumdb
I noticed that vacuum_one_database() doesn't call PQfinish() before
pg_fatal() in a few of the server version checks. I seem to have
unintentionally established this precedent in commit 00d1e88. Michael
claimed to have fixed it before committing [0]/messages/by-id/20190108020300.GH22498@paquier.xyz, but that seems to have been
missed, too. I don't think this is a huge problem, but it does seem nicer
to properly close the connection before exiting. If there are no
objections, I plan to commit and back-patch the attached patch shortly.
[0]: /messages/by-id/20190108020300.GH22498@paquier.xyz
--
nathan
Attachments:
v1-0001-vacuumdb-Add-missing-PQfinish-calls-to-vacuum_one.patchtext/plain; charset=us-asciiDownload
From be653f97707929e6b7767dc41b34c14c8b9a798b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 4 Feb 2025 10:24:09 -0600
Subject: [PATCH v1 1/1] vacuumdb: Add missing PQfinish() calls to
vacuum_one_database().
---
src/bin/scripts/vacuumdb.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 3dc428674ae..74fbc7ef033 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -557,20 +557,32 @@ vacuum_one_database(ConnParams *cparams,
}
if (vacopts->min_xid_age != 0 && PQserverVersion(conn) < 90600)
+ {
+ PQfinish(conn);
pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
"--min-xid-age", "9.6");
+ }
if (vacopts->min_mxid_age != 0 && PQserverVersion(conn) < 90600)
+ {
+ PQfinish(conn);
pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
"--min-mxid-age", "9.6");
+ }
if (vacopts->parallel_workers >= 0 && PQserverVersion(conn) < 130000)
+ {
+ PQfinish(conn);
pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
"--parallel", "13");
+ }
if (vacopts->buffer_usage_limit && PQserverVersion(conn) < 160000)
+ {
+ PQfinish(conn);
pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
"--buffer-usage-limit", "16");
+ }
/* skip_database_stats is used automatically if server supports it */
vacopts->skip_database_stats = (PQserverVersion(conn) >= 160000);
--
2.39.5 (Apple Git-154)
On 4 Feb 2025, at 17:30, Nathan Bossart <nathandbossart@gmail.com> wrote:
I noticed that vacuum_one_database() doesn't call PQfinish() before
pg_fatal() in a few of the server version checks. I seem to have
unintentionally established this precedent in commit 00d1e88. Michael
claimed to have fixed it before committing [0], but that seems to have been
missed, too. I don't think this is a huge problem, but it does seem nicer
to properly close the connection before exiting. If there are no
objections, I plan to commit and back-patch the attached patch shortly.
No objections, I too prefer to do the right thing here. +1 on backpatching.
--
Daniel Gustafsson
On Tue, Feb 04, 2025 at 05:51:28PM +0100, Daniel Gustafsson wrote:
No objections, I too prefer to do the right thing here. +1 on backpatching.
Committed. Thanks for looking.
--
nathan
On Tue, Feb 04, 2025 at 10:30:58AM -0600, Nathan Bossart wrote:
I noticed that vacuum_one_database() doesn't call PQfinish() before
pg_fatal() in a few of the server version checks. I seem to have
unintentionally established this precedent in commit 00d1e88. Michael
claimed to have fixed it before committing [0], but that seems to have been
missed, too. I don't think this is a huge problem, but it does seem nicer
to properly close the connection before exiting. If there are no
objections, I plan to commit and back-patch the attached patch shortly.
Indeed. It looks like my fingers have slipped during a rebase for
this one, or something like that. The incorrect pattern has spread
later in ae78cae3be62, not 4211fbd8413b.
Thanks for the fix, Nathan.
--
Michael