* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> =3D?UTF-8?Q?Mart=3Dc3=3Dadn_Marqu=3Dc3=3Da9s?=3D <martin@2ndquadrant.com>=
writes:
> > On HEAD I actually see that we *are* setting dump_contains as well:
>=20
> > if (fout->remoteVersion < 90600)
> > dobj->dump =3D DUMP_COMPONENT_NONE;
> > else
> > dobj->dump =3D ext->dobj.dump_contains & (DUMP_COMPONENT_ACL |
> > DUMP_COMPONENT_SECLABEL | DUMP_COMPONENT_POLICY);
>=20
> Uh, what?
>=20
> (BTW, Stephen, what's the point of the version check here? The 'else'
> code seems like it would work just fine in all branches.)
We only have the pg_init_privs information in 9.6 and above. If we
dumped out extension ACLs, etc, for prior versions, we'd end up dumping
all of them (including ones which were set initially as part of the
extension itself). That didn't seem quite right.
Generally speaking, the checks are there to keep dumps of prior-to-9.6
versions of PG similar to pre-9.6 pg_dump.
> >> So maybe just moving the checkExtensionMembership call to after the ot=
her
> >> stuff would be enough in HEAD?
>=20
> > Looking into older versions as well I can't see how placing
> > checkExtensionMembership at the end of selectDumpableNamespace would
> > help. (like-wise for other selectDumpable's)
>=20
> The fix in HEAD will necessarily look quite a bit different from
> the back branches, I fear. But the idea I had in mind there was that
> we'd calculate a namespace's dump and dump_contains flags as if it
> were not an extension member, and then allow checkExtensionMembership
> to overwrite the dump flags if it is a member. Since
> checkExtensionMembership doesn't touch the dump_contains flags, that
> would leave all the flags in the desired state.
That sounds like a pretty reasonable idea. The devil is in the details
with pg_dump though, we'll definitely want a bunch of regression tests
around this to ensure we have the correct behavior, and notice if we
break it (as happened, apparently, in the back-branches already).
Thanks!
Stephen