On Mon, Jul 6, 2026 at 4:03 AM Ayush Tiwari <[email protected]> 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
