On Fri, May 24, 2024 at 4:00 PM Tom Lane wrote:
> > Oh! That does seem like it would make what I said wrong, but how would
> > it even know who the original owner was? Shouldn't we be recreating
> > the object with the owner it had at dump time?
>
> Keep in mind that the whole point here is for
On Tue, May 28, 2024 at 9:06 PM Hannu Krosing wrote:
> We should definitely also fix pg_dump, likely just checking that the
> role exists when generating REVOKE commands (may be a good practice
> for other cases too so instead of casting to ::regrole do the actual
> join)
>
> ## here is the fix
Hi Daniel,
pg_upgrade is just one important user of pg_dump which is the one that
generates REVOKE for a non-existent role.
We should definitely also fix pg_dump, likely just checking that the
role exists when generating REVOKE commands (may be a good practice
for other cases too so instead of
> On 26 May 2024, at 23:25, Tom Lane wrote:
>
> Hannu Krosing writes:
>> Attached is a minimal patch to allow missing roles in REVOKE command
>
> FTR, I think this is a very bad idea.
Agreed, this is papering over a bug. If we are worried about pg_upgrade it
would be better to add a check to
Hannu Krosing writes:
> Attached is a minimal patch to allow missing roles in REVOKE command
FTR, I think this is a very bad idea.
It might be OK if we added some kind of IF EXISTS option,
but I'm not eager about that concept either.
The right thing here is to fix the backend so that pg_dump
Attached is a minimal patch to allow missing roles in REVOKE command
This should fix the pg_upgrade issue and also a case where somebody
has dropped a role you are trying to revoke privileges from :
smalltest=# create table revoketest();
CREATE TABLE
smalltest=# revoke select on revoketest from
On Sat, May 25, 2024 at 4:48 PM Tom Lane wrote:
>
> Hannu Krosing writes:
> > Having an pg_init_privs entry referencing a non-existing user is
> > certainly of no practical use.
>
> Sure, that's not up for debate. What I think we're discussing
> right now is
>
> 1. What other cases are badly
Hannu Krosing writes:
> Having an pg_init_privs entry referencing a non-existing user is
> certainly of no practical use.
Sure, that's not up for debate. What I think we're discussing
right now is
1. What other cases are badly handled by the pg_init_privs
mechanisms.
2. How much of that is
On Fri, May 24, 2024 at 10:00 PM Tom Lane wrote:
>
> Robert Haas writes:
> > On Fri, May 24, 2024 at 2:57 PM Tom Lane wrote:
> >> Doesn't seem right to me. That will give pg_dump the wrong idea
> >> of what the initial privileges actually were, and I don't see how
> >> it can construct correct
Robert Haas writes:
> On Fri, May 24, 2024 at 2:57 PM Tom Lane wrote:
>> Doesn't seem right to me. That will give pg_dump the wrong idea
>> of what the initial privileges actually were, and I don't see how
>> it can construct correct delta GRANT/REVOKE on the basis of false
>> information.
On Fri, May 24, 2024 at 2:57 PM Tom Lane wrote:
> Doesn't seem right to me. That will give pg_dump the wrong idea
> of what the initial privileges actually were, and I don't see how
> it can construct correct delta GRANT/REVOKE on the basis of false
> information. During the dump reload, the
Robert Haas writes:
> On Fri, May 24, 2024 at 11:59 AM Tom Lane wrote:
>> So this goal seems to
>> mean that neither ALTER OWNER nor REASSIGN OWNED should touch
>> pg_init_privs at all, as that would break its function of recording
>> a historical state. Only DROP OWNED should get rid of
On Fri, May 24, 2024 at 11:59 AM Tom Lane wrote:
> Thinking about this some more: the point of pg_init_privs is to record
> an object's privileges as they stood at the end of CREATE EXTENSION
> (or extension update), with the goal that pg_dump should be able to
> compute the delta between that
Daniel Gustafsson writes:
> On 24 May 2024, at 16:20, Tom Lane wrote:
>> Another point: shdepReassignOwned explicitly does not touch grants
>> or default ACLs. It feels like the same should be true of
>> pg_init_privs entries,
> Agreed, I can't see why pg_init_privs should be treated
> On 24 May 2024, at 16:20, Tom Lane wrote:
> I've tentatively concluded that I shouldn't have modeled
> SHARED_DEPENDENCY_INITACL so closely on SHARED_DEPENDENCY_ACL,
> in particular the decision that we don't need such an entry if
> there's also SHARED_DEPENDENCY_OWNER.
+1, in light of this
Daniel Gustafsson writes:
> I had a look, but I didn't beat you to a fix since it's not immediately clear
> to me how this should work for REASSING OWNED (DROP OWNED seems a simpler
> case). Should REASSIGN OWNED alter the rows in pg_shdepend matching init
> privs
> from SHARED_DEPENDENCY_OWNER
> On 24 May 2024, at 01:01, Tom Lane wrote:
>
> Hannu Krosing writes:
>> While the 'DROP OWNED BY fails to clean out pg_init_privs grants'
>> issue is now fixed,we have a similar issue with REASSIGN OWNED BY that
>> is still there:
>
> Ugh, how embarra
Hannu Krosing writes:
> While the 'DROP OWNED BY fails to clean out pg_init_privs grants'
> issue is now fixed,we have a similar issue with REASSIGN OWNED BY that
> is still there:
Ugh, how embarrassing. I'll take a look tomorrow, if no one
beats me to it.
reg
While the 'DROP OWNED BY fails to clean out pg_init_privs grants'
issue is now fixed,we have a similar issue with REASSIGN OWNED BY that
is still there:
Tested on fresh git checkout om May 20th
test=# create user privtestuser superuser;
CREATE ROLE
test=# set role privtestuser;
SET
test=# create
On Monday, April 29, 2024, Tom Lane wrote:
> "David G. Johnston" writes:
> > My solution to this was to rely on the fact that the bootstrap superuser
> is
> > assigned OID 10 regardless of its name.
>
> Yeah, I wrote it that way to start with too, but reconsidered
> because
>
> (1) I don't like
"David G. Johnston" writes:
> My solution to this was to rely on the fact that the bootstrap superuser is
> assigned OID 10 regardless of its name.
Yeah, I wrote it that way to start with too, but reconsidered
because
(1) I don't like hard-coding numeric OIDs. We can avoid that in C
code but
On Monday, April 29, 2024, Tom Lane wrote:
> Daniel Gustafsson writes:
> >> On 28 Apr 2024, at 20:52, Tom Lane wrote:
>
>
> >> This is of course not bulletproof: with a sufficiently weird
> >> bootstrap superuser name, we could get false matches to parts
> >> of "regress_dump_test_role" or to
I wrote:
> Pushed, thanks for reviewing!
Argh, I forgot I'd meant to push b0c5b215d first not second.
Oh well, it was only neatnik-ism that made me want to see
those other animals fail --- and a lot of the buildfarm is
red right now for $other_reasons anyway.
regards, tom
Daniel Gustafsson writes:
> On 29 Apr 2024, at 21:15, Tom Lane wrote:
>> v3 attached also has a bit more work on code comments.
> LGTM.
Pushed, thanks for reviewing!
regards, tom lane
> On 29 Apr 2024, at 21:15, Tom Lane wrote:
> It occurred to me to use "aclexplode" to expand the initprivs, and
> then we can substitute names with simple equality tests. The test
> query is a bit more complicated, but I feel better about it.
Nice, I didn't even remember that function
Daniel Gustafsson writes:
>> On 28 Apr 2024, at 20:52, Tom Lane wrote:
>> ... It's a little bit
>> nasty to look at the ACL column of pg_init_privs, because that text
>> involves the bootstrap superuser's name which is site-dependent.
>> What I did to try to make the test stable is
>>
> On 28 Apr 2024, at 20:52, Tom Lane wrote:
>
> I wrote:
>> Here's a draft patch that attacks that. It seems to fix the
>> problem with test_pg_dump: no dangling pg_init_privs grants
>> are left behind.
Reading this I can't find any sharp edges, and I prefer your changes to
I wrote:
> Here's a draft patch that attacks that. It seems to fix the
> problem with test_pg_dump: no dangling pg_init_privs grants
> are left behind.
Here's a v2 that attempts to add some queries to test_pg_dump.sql
to provide visual verification that pg_shdepend and pg_init_privs
are updated
I wrote:
> A bigger problem though is that I think you are addressing the
> original complaint from the older thread, which while it's a fine
> thing to fix seems orthogonal to the failure we're seeing in the
> buildfarm. The buildfarm's problem is not that we're recording
> incorrect
Daniel Gustafsson writes:
> On 21 Apr 2024, at 23:08, Tom Lane wrote:
>> So the meson animals are not running the test that sets up the
>> problematic data.
> 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
> On 21 Apr 2024, at 23:08, Tom Lane wrote:
>
> Daniel Gustafsson writes:
>>> On 6 Apr 2024, at 01:10, Tom Lane 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
> On 21 Apr 2024, at 23:08, Tom Lane wrote:
> ... were you going to look at it? I can take a whack if it's too far down
> your priority list.
Yeah, I’m working on a patchset right now.
./daniel
Daniel Gustafsson writes:
>> On 6 Apr 2024, at 01:10, Tom Lane 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
> On 6 Apr 2024, at 01:10, Tom Lane 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
Noah Misch writes:
> On Fri, Apr 05, 2024 at 07:10:59PM -0400, Tom Lane wrote:
>> The fact that the DROP ROLE added by 936e3fa37 succeeded indicates
>> that these role references weren't captured in pg_shdepend.
>> I imagine that we also lack code that would allow DROP OWNED BY to
>> follow up on
On Fri, Apr 05, 2024 at 07:10:59PM -0400, Tom Lane wrote:
> I wondered why buildfarm member copperhead has started to fail
> xversion-upgrade-HEAD-HEAD tests. I soon reproduced the problem here:
>
> pg_restore: creating ACL "regress_pg_dump_schema.TYPE "test_type""
> pg_restore: while PROCESSING
I wondered why buildfarm member copperhead has started to fail
xversion-upgrade-HEAD-HEAD tests. I soon reproduced the problem here:
pg_restore: creating ACL "regress_pg_dump_schema.TYPE "test_type""
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 4355; 0 0 ACL TYPE "test_type"
37 matches
Mail list logo
