On Wed, 16 Sep 2026 10:30:44 -0500
Sami Imseih <samimseih.pg@gmail.com> wrote:
> > On Tue, Sep 15, 2026 at 4:43 AM Sami Imseih <samimseih.pg@gmail.com> 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 <nagata@sraoss.co.jp>
