MIME-Version: 1.0
References: 
 <CAOYmi+=KTLd+XsEP=TDiZ48iVf-CEc7JrZd5uhWPYWKEfOgyyQ@mail.gmail.com>
 <CAOYmi+n_hw=SC5V1i3BmqfZPfPBRaUSJc+BeOXEKDwRue+WYrg@mail.gmail.com>
 <CAJTYsWXbrewUuiMekCmah3V3fA+ZvcASyV6CJgaUQ2Wc-M0MLw@mail.gmail.com>
 <CAOYmi+nns1VaW6W7kTJBTCTYVSfacJaZKyj-qxRuquT7082+WA@mail.gmail.com>
In-Reply-To: 
 <CAOYmi+nns1VaW6W7kTJBTCTYVSfacJaZKyj-qxRuquT7082+WA@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Thu, 9 Jul 2026 20:18:19 +0530
Message-ID: 
 <CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com>
Subject: Re: REVOKE's CASCADE protection doesn't work with INHERITed table
 owners
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000b173f206562eb7f2"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWUvyQchDAA6y2a9YdLcApG%3DccArpsbr77FeNZyx40bnmQ%40mail.gmail.com>
Precedence: bulk

--000000000000b173f206562eb7f2
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, 9 Jul 2026 at 04:24, Jacob Champion <jacob.champion@enterprisedb.co=
m>
wrote:

> On Mon, Jul 6, 2026 at 4:03=E2=80=AFAM Ayush Tiwari <ayushtiwari.slg01@gm=
ail.com>
> wrote:
>
> 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...
>

Fair. The part that tripped me up was actually the second sentence,
"granted by any role on the chain", since the code isn't checking whether
some role could grant them, but whether the grantee itself still holds
them directly via another grantor. Maybe something like "if the grantee
still holds a grant option directly through another grantor, that
privilege's chain is intact"? No strong opinion though.


> > 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 o=
f
> > 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?
>

The already-failing case in [1] is the one where the membership has
been revoked (REVOKE member FROM owner), which leaves a true orphan.
But the self-grant gets created as soon as an inheriting member does
the redundant WITH GRANT OPTION self-grant, and if the membership is
still in place, that cluster dumps and restores fine today.

  CREATE ROLE owner_role;
  CREATE ROLE member_role LOGIN;
  GRANT owner_role TO member_role;          -- membership kept
  CREATE TABLE t (i int);
  ALTER TABLE t OWNER TO owner_role;
  GRANT SELECT ON t TO member_role WITH GRANT OPTION;
  SET ROLE member_role;
  GRANT SELECT ON t TO member_role WITH GRANT OPTION;   -- self-grant

  relacl =3D {owner_role=3DarwdDxtm/owner_role,
            member_role=3Dr*/owner_role,
            member_role=3Dr*/member_role}

pg_dump emits "SET SESSION AUTHORIZATION member_role; GRANT SELECT ON
t TO member_role WITH GRANT OPTION;". Restoring that into a fresh
cluster works today, but with the check_circularity() change it fails
with "grant options cannot be granted back to your own grantor".

So it's not that [1] is fine today; it's that the fix also breaks a
broader set of clusters that currently round-trip (membership still
present), including via pg_upgrade. That's why I think the
check_circularity() side needs a companion dump/restore change rather
than going in on its own.


> 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.
>

Agreed, I did not check that either.

Regards,
Ayush

--000000000000b173f206562eb7f2
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, 9 Jul 20=
26 at 04:24, Jacob Champion &lt;<a href=3D"mailto:jacob.champion@enterprise=
db.com">jacob.champion@enterprisedb.com</a>&gt; wrote:<br></div><blockquote=
 class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px so=
lid rgb(204,204,204);padding-left:1ex">On Mon, Jul 6, 2026 at 4:03=E2=80=AF=
AM Ayush Tiwari &lt;<a href=3D"mailto:ayushtiwari.slg01@gmail.com" target=
=3D"_blank">ayushtiwari.slg01@gmail.com</a>&gt; wrote:<br>
<br>
The first sentence is still &quot;The grantee might still have some grant<b=
r>
options via another grantor,&quot; and I don&#39;t think adding &quot;direc=
t&quot; would<br>
have helped me understand this any better the first time I read it.<br>
I&#39;m definitely up for more bikeshedding, though, because I don&#39;t<br=
>
really like what I have...<br></blockquote><div><br>Fair. The part that tri=
pped me up was actually the second sentence,<br>&quot;granted by any role o=
n the chain&quot;, since the code isn&#39;t checking whether<br>some role c=
ould grant them, but whether the grantee itself still holds<br>them directl=
y via another grantor. Maybe something like &quot;if the grantee<br>still h=
olds a grant option directly through another grantor, that<br>privilege&#39=
;s chain is intact&quot;? No strong opinion though.<br>=C2=A0</div><blockqu=
ote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px=
 solid rgb(204,204,204);padding-left:1ex">
&gt; On check_circularity() for [1]: I tried the same aclmask_direct() swap=
,<br>
&gt; but since it runs on every GRANT ... WITH GRANT OPTION, which<br>
&gt; pg_dump/restore replays, erroring there could make restore/pg_upgrade =
of<br>
&gt; an existing cluster (one already holding the [1] self-grant) fail.<br>
<br>
Well, I think the complaint in [1] is that dump/restore *already*<br>
fails, no? Can you provide an example of a correct (or benignly<br>
incorrect) dump that would start failing?<br></blockquote><div><br>The alre=
ady-failing case in [1] is the one where the membership has<br>been revoked=
 (REVOKE member FROM owner), which leaves a true orphan.<br>But the self-gr=
ant gets created as soon as an inheriting member does<br>the redundant WITH=
 GRANT OPTION self-grant, and if the membership is<br>still in place, that =
cluster dumps and restores fine today.<br><br>=C2=A0 CREATE ROLE owner_role=
;<br>=C2=A0 CREATE ROLE member_role LOGIN;<br>=C2=A0 GRANT owner_role TO me=
mber_role; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-- membership kept<br>=C2=A0 C=
REATE TABLE t (i int);<br>=C2=A0 ALTER TABLE t OWNER TO owner_role;<br>=C2=
=A0 GRANT SELECT ON t TO member_role WITH GRANT OPTION;<br>=C2=A0 SET ROLE =
member_role;<br>=C2=A0 GRANT SELECT ON t TO member_role WITH GRANT OPTION; =
=C2=A0 -- self-grant<br><br>=C2=A0 relacl =3D {owner_role=3DarwdDxtm/owner_=
role,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 member_role=3Dr*/owner_r=
ole,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 member_role=3Dr*/member_r=
ole}<br><br>pg_dump emits &quot;SET SESSION AUTHORIZATION member_role; GRAN=
T SELECT ON<br>t TO member_role WITH GRANT OPTION;&quot;. Restoring that in=
to a fresh<br>cluster works today, but with the check_circularity() change =
it fails<br>with &quot;grant options cannot be granted back to your own gra=
ntor&quot;.<br><br>So it&#39;s not that [1] is fine today; it&#39;s that th=
e fix also breaks a<br>broader set of clusters that currently round-trip (m=
embership still<br>present), including via pg_upgrade. That&#39;s why I thi=
nk the<br>check_circularity() side needs a companion dump/restore change ra=
ther<br>than going in on its own.<br>=C2=A0</div><blockquote class=3D"gmail=
_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204=
,204);padding-left:1ex">
As an aside, I&#39;m not sure if check_circularity() is correctly<br>
preventing cycles independently of this issue, so that part may end up<br>
spiraling a bit.<br></blockquote><div><br>Agreed, I did not check that eith=
er.<br><br>Regards,<br>Ayush=C2=A0</div></div></div>

--000000000000b173f206562eb7f2--