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
