Re: DROP OWNED BY fails to clean out pg_init_privs grants

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: DROP OWNED BY fails to clean out pg_init_privs grants
Date: 2024-04-26 13:41:41
Message-ID: F27F3F6F-A052-4EEF-9A11-3D10A867843F@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 21 Apr 2024, at 23:08, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> On 6 Apr 2024, at 01:10, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> (So now I'm wondering why *only* copperhead has shown this so far.
>>> Are our other cross-version-upgrade testing animals AWOL?)
>
>> Clicking around searching for Xversion animals I didn't spot any, but without
>> access to the database it's nontrivial to know which animal does what.
>
> I believe I see why this is (or isn't) happening. The animals
> currently running xversion tests are copperhead, crake, drongo,
> and fairywren. copperhead is using the makefiles while the others
> are using meson. And I find this in
> src/test/modules/test_pg_dump/meson.build (from 3f0e786cc):
>
> # doesn't delete its user
> 'runningcheck': false,
>
> So the meson animals are not running the test that sets up the
> problematic data.

ugh =/

> I think we should remove the above, since (a) the reason to have
> it is gone, and (b) it seems really bad that the set of tests
> run by meson is different from that run by the makefiles.

Agreed.

> However, once we do that, those other three animals will presumably go
> red, greatly complicating detection of any Windows-specific problems.
> So I'm inclined to not do it till just before we intend to commit
> a fix for the underlying problem. (Enough before that we can confirm
> that they do go red.)

Agreed, we definitely want that but compromising the ability to find Windows
issues at this point in the cycle seems bad.

> ... were you going to look at it? I can take a whack if it's
> too far down your priority list.

I took a look at this, reading code and the linked thread. My gut feeling is
that Stephen is right in that the underlying bug is these privileges ending up
in pg_init_privs to begin with. That being said, I wasn't able to fix that in
a way that doesn't seem like a terrible hack. The attached POC hack fixes it
for me but I'm not sure how to fix it properly. Your wisdom would be much appreciated.

Clusters which already has such entries aren't helped by a fix for this though,
fixing that would either require pg_dump to skip them, or pg_upgrade to have a
check along with instructions for fixing the issue. Not sure what's the best
strategy here, the lack of complaints could indicate this isn't terribly common
so spending cycles on it for every pg_dump might be excessive compared to a
pg_upgrade check?

--
Daniel Gustafsson

Attachment Content-Type Size
init_privs.diff application/octet-stream 4.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2024-04-26 13:48:45 Re: New committers: Melanie Plageman, Richard Guo
Previous Message Aleksander Alekseev 2024-04-26 13:41:18 Re: New committers: Melanie Plageman, Richard Guo