pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedadd missing PQfinish() calls to vacuumdb
4+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2025-02-04 f3e4aeb744 d6ea82f5da 8703cdec20 e8d8174caf 8c2dd212d1 eeaf6891be vacuumdb: Add missing PQfinish() calls to vacuum_one_database().
* add missing PQfinish() calls to vacuumdb
@ 2025-02-04 16:30 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 4+ messages in thread
From: Nathan Bossart @ 2025-02-04 16:30 UTC (permalink / raw)
To: pgsql-hackers
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.
[0] https://postgr.es/m/20190108020300.GH22498%40paquier.xyz
--
nathan
Attachments:
v1-0001-vacuumdb-Add-missing-PQfinish-calls-to-vacuum_one.patch (text/plain, 1.6K) download
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)
* Re: add missing PQfinish() calls to vacuumdb
@ 2025-02-04 16:51 Daniel Gustafsson <daniel@yesql.se>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Gustafsson @ 2025-02-04 16:51 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
> 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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: add missing PQfinish() calls to vacuumdb
@ 2025-02-04 19:30 Nathan Bossart <nathandbossart@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 4+ messages in thread
From: Nathan Bossart @ 2025-02-04 19:30 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: add missing PQfinish() calls to vacuumdb
@ 2025-02-05 00:36 Michael Paquier <michael@paquier.xyz>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2025-02-05 00:36 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
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.
>
> [0] https://postgr.es/m/20190108020300.GH22498%40paquier.xyz
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2025-02-05 00:36 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-02-04 16:30 add missing PQfinish() calls to vacuumdb Nathan Bossart <nathandbossart@gmail.com>
2025-02-04 16:51 ` Daniel Gustafsson <daniel@yesql.se>
2025-02-04 19:30 ` Nathan Bossart <nathandbossart@gmail.com>
2025-02-05 00:36 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox