From: Antonin Houska <ah@cybertec.at>
To: alvherre@kurilemu.de
cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: splitting src/bin/scripts/vacuumdb.c
In-reply-to: <202509261126.lqdaq6uvo2m2@alvherre.pgsql>
References: <202509261126.lqdaq6uvo2m2@alvherre.pgsql>
Comments: In-reply-to =?us-ascii?Q?=3D=3Futf-8=3FQ=3F=3DC3=3D81lvaro=3F=3D?=
 Herrera <alvherre@kurilemu.de>
   message dated "Fri, 26 Sep 2025 13:55:50 +0200."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Fri, 26 Sep 2025 15:46:35 +0200
Message-ID: <48811.1758894395@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/48811.1758894395%40localhost>
Precedence: bulk

=C3=81lvaro Herrera <alvherre@kurilemu.de> wrote:

> - Two booleans, analyze_in_stages and analyze_only, are really
> determining what "mode" the program runs in -- it's either vacuum, or
> one of those two modes.  (Antonin came up with the idea of using
> "modes", but his patch only adds a new REPACK mode on top of the
> existing code without any further changes).  I think the code flow is a
> little neater by making this change; consider for instance this change:
>=20
> -   /* If we used SKIP_DATABASE_STATS, mop up with ONLY_DATABASE_STATS */
> -   if (vacopts->skip_database_stats && stage =3D=3D ANALYZE_NO_STAGE &&
> -       !vacopts->analyze_only)
> -   {
>=20
> +   /* If we used SKIP_DATABASE_STATS, mop up with ONLY_DATABASE_STATS */
> +   if (vacopts->mode =3D=3D MODE_VACUUM && vacopts->skip_database_stats)
> +   {
>=20
> IMO the original is quite unclear.

I agree that redundant information makes things more difficult to think
about. I just wonder if

	vacopts->mode !=3D MODE_VACUUM

should be used instead of

	(vacopts->mode =3D=3D MODE_ANALYZE ||
	vacopts->mode =3D=3D MODE_ANALYZE_IN_STAGES)

in some places. Nevertheless, I'm not proposing a global replacement: the
verbose form may be easier to understand, depending on the context.

Other than that, I checked differences between v21, v22 and v23. I've got no
other comments worth posting.

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com