Jacob Champion <jchamp...@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