From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jacob Champion <jchampion@timescale.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Akshat Jaimini <destrex271@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump needs SELECT privileges on irrelevant extension table
Date: Tue, 17 Oct 2023 16:12:52 -0400
Message-ID: <2905669.1697573572@sss.pgh.pa.us> (raw)
In-Reply-To: <CAAWbhmi+fu6Aiucjf76Ss2GPEijrz6VLriby1LaG=p9zNTOxZg@mail.gmail.com>
References: <00d46a48-3324-d9a0-49bf-e7f0f11d1038@timescale.com>
<954419.1679334206@sss.pgh.pa.us>
<eed220a4-c7f3-2164-02f2-b42783694221@timescale.com>
<a185f66f-1edc-426e-41a4-091dad230c75@dunslane.net>
<CAAWbhmjsm+bJrwC1eTGpGk+6MQFG_1vA2g1+Fqa0xE78v1Xncg@mail.gmail.com>
<CAAWbhmi+fu6Aiucjf76Ss2GPEijrz6VLriby1LaG=p9zNTOxZg@mail.gmail.com>
Jacob Champion <jchampion@timescale.com> writes:
> v3 fixes a doc comment I forgot to fill in; there are no other code
> changes. To try to further reduce the activation energy, I've also
> attached an attempt at a backport to 11. The main difference is the
> absence of catalogIdHash, which showed up in 15, so we don't get the
> benefit of that deduplication.
So ... I still do not like anything about this patch. Putting
has_policies into CatalogIdMapEntry isn't a wart, it's more
nearly a tumor. Running getTablesWithPolicies before we can
acquire locks is horrid from the standpoint of minimizing the
window between our transaction snapshot and successful acquisition
of all needed locks. (It might be all right in databases with
few pg_policy entries, but I don't think we can assume that that
holds everywhere.) And the whole thing is just ugly and solves
the problem only partially.
What I am wondering about is whether we shouldn't just undo what
checkExtensionMembership does, specifically:
/*
* In 9.6 and above, mark the member object to have any non-initial ACL,
* policies, and security labels dumped.
*
* Note that any initial ACLs (see pg_init_privs) will be removed when we
* extract the information about the object. We don't provide support for
* initial policies and security labels and it seems unlikely for those to
* ever exist, but we may have to revisit this later.
*
* ...
*/
dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL |
DUMP_COMPONENT_SECLABEL |
DUMP_COMPONENT_POLICY);
Why are we marking extension member objects as being subject to SECLABEL
or POLICY dumping? As the comment notes, that isn't really sensible
unless what we are dumping is a delta from the extension's initial
assignments. But we have no infrastructure for that, and none seems
likely to appear in the near future.
Could we not fix this by just reducing the above to
dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL);
When and if someone comes along and implements storage of extensions'
initial policies, they can figure out how to avoid fetching policies for
not-to-be-dumped tables. (My thoughts would run towards adding a column
to pg_class to help detect whether such work is needed without doing
expensive additional queries.) But I don't see why we require a solution
to that problem as things stand.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, jchampion@timescale.com, sfrost@snowman.net, andrew@dunslane.net, destrex271@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_dump needs SELECT privileges on irrelevant extension table
In-Reply-To: <2905669.1697573572@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox