On Wed, 16 Sep 2026 10:30:44 -0500
Sami Imseih <[email protected]> wrote:

> > On Tue, Sep 15, 2026 at 4:43 AM Sami Imseih <[email protected]> wrote:
> > > This is less about stats reporting and more about filtering out such
> > > relations earlier in expand_vacuum_rel() for manually specified
> > > VACUUM/ANALYZE targets. Not a big thing, but it would avoid an extra
> > > syscache lookup and a misleading "lock not available" warning.
> >
> > I would like to fold one more case into that follow-up. The skip is
> > reported before vacuum_is_permitted_for_relation(), so a user with no
> > rights on the table can still move its counters.
> >
> > The callback argument of RangeVarGetRelidExtended() looks like the
> > right place for all of this. It runs before the lock attempt, and the
> > comment there already says that is where permission checks belong. It
> > also hands us relId, so expand_vacuum_rel() would no longer need the
> > second RangeVarGetRelid(..., NoLock, true) lookup.
> 
> It may be worth cleaning this stuff up if someone is interested in taking
> this on.
> 
> v16 is just a rebase. I also took out the catversion change for now, but made
> it clear it's required in the commit message.

Thank you for updating the patch.
I'm fine with the changes you made incorporating Kato-san's suggestions.

One minor comment is:

                toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
+               toast_vacuum_params.options &= ~VACOPT_ANALYZE;
                toast_vacuum_params.toast_parent = relid;

I wonder if this change is still necessary now that the filtering for TOAST
tables has been added to pgstat_report_skipped_vacuum_analyze().

If we keep this change, should we also add a comment explaining
why it is needed?

Regards,
Yugo Nagata

-- 
Yugo Nagata <[email protected]>


Reply via email to