On Mon, Jul 6, 2026 at 4:03 AM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
> Thanks, this looks right to me.
Thanks for the review!
> One tiny comment question: the phrase "granted by any role on the chain"
> in the new comment reads a little oddly to me, would something like
> "still holds the option directly via another grantor" be closer to what
> the code checks?
The first sentence is still "The grantee might still have some grant
options via another grantor," and I don't think adding "direct" would
have helped me understand this any better the first time I read it.
I'm definitely up for more bikeshedding, though, because I don't
really like what I have...
> On check_circularity() for [1]: I tried the same aclmask_direct() swap,
> but since it runs on every GRANT ... WITH GRANT OPTION, which
> pg_dump/restore replays, erroring there could make restore/pg_upgrade of
> an existing cluster (one already holding the [1] self-grant) fail.
Well, I think the complaint in [1] is that dump/restore *already*
fails, no? Can you provide an example of a correct (or benignly
incorrect) dump that would start failing?
As an aside, I'm not sure if check_circularity() is correctly
preventing cycles independently of this issue, so that part may end up
spiraling a bit.
Thanks,
--Jacob
