Hi all,
v8 is a better landing than v6 (which I'd LGTM'd). Michael's repro made it clear that removing the early ACL filter would let
an unprivileged VACUUM actually stack behind a privileged lock, which a556549 commit was preventing.
v8 preserves that protection while handling the concurrent-drop in get_all_vacuum_rels, and feels like the right shape.
A few small notes on the patch:
1. The function header comment for vacuum_is_permitted_for_relation() is unchanged and still describes only
two return paths: "issue a WARNING log message and return false". v8 introduces a third path that returns false when
is_missing fires. Worth updating the comment to describe all three return cases and what the new missing_ok parameter controls.
2. The silent skip in get_all_vacuum_rels produces a different user-visible behavior than vacuum_open_relation's WARNING for
what's essentially the same race (concurrent drop during a database-wide VACUUM).
I think the silent path is fine here, as the user didn't explicitly ask for that table.
3. The Assert that Bharath suggested earlier doesn't apply as-is but can be incorporated into the patch as:
Assert(missing_ok || CheckRelationOidLockedByMe(relid, AccessShareLock, true));
That would catch any future caller that passes missing_ok=false without first acquiring a lock.
Regards,
Surya Poondla
