Re: pg_dump needs SELECT privileges on irrelevant extension table
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jacob Champion <jchampion@timescale.com>
Cc: Stephen Frost <sfrost@snowman.net>, Andrew Dunstan <andrew@dunslane.net>,
Akshat Jaimini <destrex271@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-10-17T20:12:52Z
Lists: pgsql-bugs, pgsql-hackers
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
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Don't try to dump RLS policies or security labels for extension objects.
- a70f2a57f233 17.0 landed
- 64d2467fc8d3 16.2 landed
- f15147df625f 15.6 landed
- 9e08789d4832 13.14 landed
- 891afa84c27e 12.18 landed
- 15439205d8ce 14.11 landed