Re: pgsql: Move various prechecks from vacuum() into ExecVacuum()

Hi,

On 2023-04-06 13:04:17 +0900, Michael Paquier wrote:
> On Thu, Apr 06, 2023 at 03:45:19AM +0000, David Rowley wrote:
> > Move various prechecks from vacuum() into ExecVacuum()
> >
> > vacuum() is used for both the VACUUM command and for autovacuum. There
> > were many prechecks being done inside vacuum() that were just not relevant
> > to autovacuum. Let's move the bulk of these into ExecVacuum() so that
> > they're only executed when running the VACUUM command. This removes a
> > small amount of overhead when autovacuum vacuums a table.
> >
> > While we are at it, allocate VACUUM's BufferAccessStrategy in ExecVacuum()
> > and pass it into vacuum() instead of expecting vacuum() to make it if it's
> > not already made by the calling function. To make this work, we need to
> > create the vacuum memory context slightly earlier, so we now need to pass
> > that down to vacuum() so that it's available for use in other memory
> > allocations.
>
> I have just seen this commit, and I am pretty sure that the checks
> have been placed in vacuum() to guard against incorrect option
> manipulations in the context of an autovacuum building the relations,
> so you are making this code weaker with the cross-checks it had, IMO.
> This is particularly relevant in some areas with toast relations, for
> example, because autovacuum handles the toast and their parents
> separately, contrary to ExecVacuum() that would group them together by
> default.

They're prety useless for that though. You don't even see those errors
anywhere. For the purpose of validating that autovac does something sensible
you just need an Assert(). And it can be in do_autovacuum() or in
vacuum(). You don't need translated ereport()s.

Greetings,

Andres Freund