Date: Fri, 26 Sep 2025 09:15:25 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: splitting src/bin/scripts/vacuumdb.c
Message-ID: <aNaf_YP9ruHG4Zbp@nathan>
References: <202509261126.lqdaq6uvo2m2@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202509261126.lqdaq6uvo2m2@alvherre.pgsql>
Archived-At: <https://www.postgresql.org/message-id/aNaf_YP9ruHG4Zbp%40nathan>
Precedence: bulk

On Fri, Sep 26, 2025 at 01:55:50PM +0200, Álvaro Herrera wrote:
> - objfilter is declared as an enum but the values are bit-or'ed and
> tested for individual bits.  We've discussed this coding pattern in
> other contexts and stayed away from it, on the grounds that the values
> so generated aren't really true values of the enum.  I changed it from
> an enum to a bits32 with a few #defines for the values we use, the way
> we do elsewhere.  Also, instead of being passed as a separate argument
> to the various functions, I added it to the vacuumingOpts struct.
> 
> - 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:

Seems reasonable to me.

> Overall I'm quite happy with this small patch now, so I intend to push
> shortly barring objections.  I'm not adding a new commitfest entry, just
> adding this new thread to the existing entry for REPACK.  I kept the
> patch numbering as used in the other thread.

Is there any way to separate the aforementioned changes into a separate
patch from the refactoring changes?  That would make it a lot easier to
review.  I think we have decent testing for vacuumdb, so I'm not too
concerned if this isn't feasible.

-- 
nathan