Thread: pg_dump doesn't dump new objects created in schemas from extensions
I'm filling this bug report as I believe pg_dump is not generating a
consistent backup when some extensions are being used (particularly,
ones that create schemas, like PgQ).
Here is an example:
pruebas=# create extension pgq;
CREATE EXTENSION
pruebas=# select pgq.create_queue('personas');create_queue
-------------- 1
(1 fila)
pruebas=# select pgq.create_queue('usuarios');create_queue
-------------- 1
(1 fila)
pruebas=# select pgq.create_queue('usuarios_activos');create_queue
-------------- 1
(1 fila)
pruebas=# select pgq.create_queue('usuarios_inactivos');create_queue
-------------- 1
(1 fila)
pruebas=# select count(*) from pgq.tick;count
------- 4
(1 fila)
pruebas=# \dt pgq.* Listado de relacionesEsquema | Nombre | Tipo | Dueño
---------+----------------+-------+----------pgq | consumer | tabla | postgrespgq | event_1 |
tabla| postgrespgq | event_1_0 | tabla | postgrespgq | event_1_1 | tabla | postgrespgq |
event_1_2 | tabla | postgrespgq | event_2 | tabla | postgrespgq | event_2_0 | tabla |
postgrespgq | event_2_1 | tabla | postgrespgq | event_2_2 | tabla | postgrespgq | event_3 |
tabla| postgrespgq | event_3_0 | tabla | postgrespgq | event_3_1 | tabla | postgrespgq |
event_3_2 | tabla | postgrespgq | event_4 | tabla | postgrespgq | event_4_0 | tabla |
postgrespgq | event_4_1 | tabla | postgrespgq | event_4_2 | tabla | postgrespgq | event_template |
tabla| postgrespgq | queue | tabla | postgrespgq | retry_queue | tabla | postgrespgq |
subscription | tabla | postgrespgq | tick | tabla | postgres
(22 filas)
And just to add something else into the whole annoyance, I'll add a user
defined table:
pruebas=# create table pgq.test_pgq_dumpable (id int primary key);
CREATE TABLE
pruebas=# \dt pgq.test_pgq_dumpable Listado de relacionesEsquema | Nombre | Tipo | Dueño
---------+-------------------+-------+----------pgq | test_pgq_dumpable | tabla | postgres
(1 fila)
To check that all objects from the pgq schema were dumped, I just pipe
the pg_dump to psql on a new DB:
-bash-4.3$ pg_dump pruebas | psql -d pruebas_pgq
Now, let's check what we have on this new DB:
pruebas_pgq=# \dt pgq.test_pgq_dumpable
No se encontraron relaciones coincidentes.
pruebas_pgq=# \dt pgq.* Listado de relacionesEsquema | Nombre | Tipo | Dueño
---------+----------------+-------+----------pgq | consumer | tabla | postgrespgq | event_template |
tabla| postgrespgq | queue | tabla | postgrespgq | retry_queue | tabla | postgrespgq |
subscription | tabla | postgrespgq | tick | tabla | postgres
(6 filas)
This problem came up due to a difference between pg_dump on 9.1.12 and
9.1.22 (before is dumped all the tables and sequences and with the new
point release is doesn't anymore), but here I'm using 9.5.3:
pruebas_pgq=# select version(); version
---------------------------------------------------------------------------------------------------------PostgreSQL
9.5.3on x86_64-pc-linux-gnu, compiled by gcc (GCC) 5.3.1
20160406 (Red Hat 5.3.1-6), 64-bit
(1 fila)
Regards,
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Martín Marqués wrote: > -bash-4.3$ pg_dump pruebas | psql -d pruebas_pgq > > Now, let's check what we have on this new DB: > > pruebas_pgq=# \dt pgq.test_pgq_dumpable > No se encontraron relaciones coincidentes. > pruebas_pgq=# \dt pgq.* > Listado de relaciones > Esquema | Nombre | Tipo | Dueño > ---------+----------------+-------+---------- > pgq | consumer | tabla | postgres > pgq | event_template | tabla | postgres > pgq | queue | tabla | postgres > pgq | retry_queue | tabla | postgres > pgq | subscription | tabla | postgres > pgq | tick | tabla | postgres > (6 filas) To make the problem worse, the pgq.create_queue() function also inserts rows in the pgq.queue table. The pgq.queue table is marked as a configuration table, so the rows that reference the pgq.event_1 etc tables are dumped, but not the tables themselves. This is pretty messy and I wonder why it hadn't been reported thus far. I would argue that those secondary tables should be created in a different schema, but then that schema would probably be created by the pgq extension script too and would not get dumped either. So we want the policy to change to "dump the table unless it has a dependency on an extension" (rather than "inherit dump flag from containing schema). I wonder what other things this change would break. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
2016-06-16 15:55 GMT-03:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
>
> To make the problem worse, the pgq.create_queue() function also inserts
> rows in the pgq.queue table. The pgq.queue table is marked as a
> configuration table, so the rows that reference the pgq.event_1 etc
> tables are dumped, but not the tables themselves.
Exactly. And that's why all the pgq.get_*_info() functions error out
with a missing event table.
> This is pretty messy and I wonder why it hadn't been reported thus far.
> I would argue that those secondary tables should be created in a
> different schema, but then that schema would probably be created by the
> pgq extension script too and would not get dumped either.
I bet that most people load the sql or work with outside processes
like Londiste that creates all the needed functions and tables without
using PgQ as an extension (this is actually the first time I work with
PgQ as an extension, even though I've worked alot with it together
with Londiste.
> So we want the policy to change to "dump the table unless it has a
> dependency on an extension" (rather than "inherit dump flag from
> containing schema). I wonder what other things this change would break.
IMO, this assignment:
tbinfo->dobj.dump = tbinfo->dobj.namespace->dobj.dump_contains;
should be replaced by a call to some new function which would be more
or less a copy of selectDumpableNamespace, but without the
checkExtensionMembership call.
Regards,
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Jun 17, 2016 at 4:21 AM, Martín Marqués <martin@2ndquadrant.com> wrote: > IMO, this assignment: > > tbinfo->dobj.dump = tbinfo->dobj.namespace->dobj.dump_contains; > > should be replaced by a call to some new function which would be more > or less a copy of selectDumpableNamespace, but without the > checkExtensionMembership call. Ah, I see. Yes this is definitely wrong. The namespace itself may be part of an extension but we do not check for it at all. See for example the patch attached that is giving what I would expect is the correct behavior. -- Michael
Attachment
El 17/06/16 a las 03:35, Michael Paquier escribió: > On Fri, Jun 17, 2016 at 4:21 AM, Martín Marqués <martin@2ndquadrant.com> wrote: >> IMO, this assignment: >> >> tbinfo->dobj.dump = tbinfo->dobj.namespace->dobj.dump_contains; >> >> should be replaced by a call to some new function which would be more >> or less a copy of selectDumpableNamespace, but without the >> checkExtensionMembership call. > > Ah, I see. Yes this is definitely wrong. The namespace itself may be > part of an extension but we do not check for it at all. See for > example the patch attached that is giving what I would expect is the > correct behavior. Yes, that's the correct behavior we would normally expect, but not if schema_include_oids.head is not null, or schema_exclude_oids has the oid of the tbinfo->dobj.namespace. Regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
El 17/06/16 a las 03:35, Michael Paquier escribió: > On Fri, Jun 17, 2016 at 4:21 AM, Martín Marqués <martin@2ndquadrant.com> wrote: >> IMO, this assignment: >> >> tbinfo->dobj.dump = tbinfo->dobj.namespace->dobj.dump_contains; >> >> should be replaced by a call to some new function which would be more >> or less a copy of selectDumpableNamespace, but without the >> checkExtensionMembership call. > > Ah, I see. Yes this is definitely wrong. The namespace itself may be > part of an extension but we do not check for it at all. See for > example the patch attached that is giving what I would expect is the > correct behavior. This is a new version of your patch that checks for schema inclusion or exclusion. If there are no complains, I could apply similar changes on other objects for a more complete patch. Thoughts, objections? -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
On Sat, Jun 18, 2016 at 2:21 AM, Mart=C3=ADn Marqu=C3=A9s <martin@2ndquadra= nt.com> wrote: > El 17/06/16 a las 03:35, Michael Paquier escribi=C3=B3: >> On Fri, Jun 17, 2016 at 4:21 AM, Mart=C3=ADn Marqu=C3=A9s <martin@2ndqua= drant.com> wrote: >>> IMO, this assignment: >>> >>> tbinfo->dobj.dump =3D tbinfo->dobj.namespace->dobj.dump= _contains; >>> >>> should be replaced by a call to some new function which would be more >>> or less a copy of selectDumpableNamespace, but without the >>> checkExtensionMembership call. >> >> Ah, I see. Yes this is definitely wrong. The namespace itself may be >> part of an extension but we do not check for it at all. See for >> example the patch attached that is giving what I would expect is the >> correct behavior. > > This is a new version of your patch that checks for schema inclusion or > exclusion. > > If there are no complains, I could apply similar changes on other > objects for a more complete patch. > > Thoughts, objections? Tests, and refactoring. As this is going to be heavily duplicated, you may want to use a different routine for this check. Integrating some tests with a dummy extension that creates a schema would also be a good idea as this is likely going to modify many code paths. So I think that we should have something stored in src/test/modules that gets installed when running make check in src/bin/pg_dump, and then one of the TAP scripts in t/ creates a bunch of objects on the schema created by the extension. --=20 Michael
El 17/06/16 a las 20:31, Michael Paquier escribió: > > Tests, and refactoring. > > As this is going to be heavily duplicated, you may want to use a > different routine for this check. Integrating some tests with a dummy > extension that creates a schema would also be a good idea as this is > likely going to modify many code paths. So I think that we should have > something stored in src/test/modules that gets installed when running > make check in src/bin/pg_dump, and then one of the TAP scripts in t/ > creates a bunch of objects on the schema created by the extension. That's sensible, but my day is coming to an end. ;) I'll see if I can make some time to get all that added, and I'll also see which other objects need to be dumpable as well. Thanks for helping out with this. -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
El 17/06/16 a las 20:31, Michael Paquier escribió: > On Sat, Jun 18, 2016 at 2:21 AM, Martín Marqués <martin@2ndquadrant.com> wrote: >> >> If there are no complains, I could apply similar changes on other >> objects for a more complete patch. >> >> Thoughts, objections? > > Tests, and refactoring. > > As this is going to be heavily duplicated, you may want to use a > different routine for this check. Yes, that was my initial thought. > Integrating some tests with a dummy > extension that creates a schema would also be a good idea as this is > likely going to modify many code paths. So I think that we should have > something stored in src/test/modules that gets installed when running > make check in src/bin/pg_dump, and then one of the TAP scripts in t/ > creates a bunch of objects on the schema created by the extension. AFAICS there's a test_pg_dump module in src/test/modules which is there to test pg_dump with extensions. I think that might be the best place to have these tests. Regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
* Mart=EDn Marqu=E9s (martin@2ndquadrant.com) wrote: > El 17/06/16 a las 20:31, Michael Paquier escribi=F3: > > Integrating some tests with a dummy > > extension that creates a schema would also be a good idea as this is > > likely going to modify many code paths. So I think that we should have > > something stored in src/test/modules that gets installed when running > > make check in src/bin/pg_dump, and then one of the TAP scripts in t/ > > creates a bunch of objects on the schema created by the extension. >=20 > AFAICS there's a test_pg_dump module in src/test/modules which is there > to test pg_dump with extensions. I think that might be the best place to > have these tests. Yes, that's certainly the intent. We can't put the test in src/bin/pg_dump because the extensions aren't available for installation when those tests are being run (at least, that was the case when I tried, which is why src/test/modules/test_pg_dump exists). Thanks! Stephen
Martín Marqués <martin@2ndquadrant.com> writes:
> This is a new version of your patch that checks for schema inclusion or
> exclusion.
> If there are no complains, I could apply similar changes on other
> objects for a more complete patch.
> Thoughts, objections?
This patch seems to me to be entirely the wrong approach, as what it
will result in is an enormous amount of code duplication. There's
nothing fundamentally wrong with looking at the parent namespace to
decide whether to dump or not. The difficulty is that we need to draw
a distinction *at the namespace level* between "dump this namespace"
and "dump objects within this namespace". The correct behavior for
a namespace owned by an extension is to do the latter (for objects
not belonging to any extension) but not the former. So instead of
just one "dump" flag, we need two such flags for namespaces. The
various selectDumpableFoo routines need one-line adjustments as to
which flag they look at in the parent namespace. They do *not*
need copy-and-paste of their entire current contents.
In HEAD, this should be a pretty straightforward change since Stephen
Frost has already created infrastructure that allows distinguishing which
"components" of an object to dump. There doesn't seem to be a bit that
corresponds to "objects within this schema", but that shouldn't be hard
to add.
The DUMP_COMPONENT infrastructure doesn't exist in older branches
so I think you'd just have to invent an additional bool field in
NamespaceInfo structs to say whether to dump contained objects.
A slightly different approach is to ignore the DumpComponents stuff
and just add a bool field in NamespaceInfo for all branches. Perhaps
this would even be the right thing, as I may be misunderstanding what
Stephen intends the DumpComponents bits to be used for. Would be
interesting to hear Stephen's take on this.
regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > =3D?UTF-8?Q?Mart=3Dc3=3Dadn_Marqu=3Dc3=3Da9s?=3D <martin@2ndquadrant.com>= writes: > > This is a new version of your patch that checks for schema inclusion or > > exclusion. >=20 > > If there are no complains, I could apply similar changes on other > > objects for a more complete patch. >=20 > > Thoughts, objections? >=20 > This patch seems to me to be entirely the wrong approach, as what it > will result in is an enormous amount of code duplication. There's > nothing fundamentally wrong with looking at the parent namespace to > decide whether to dump or not. The difficulty is that we need to draw > a distinction *at the namespace level* between "dump this namespace" > and "dump objects within this namespace". The correct behavior for > a namespace owned by an extension is to do the latter (for objects > not belonging to any extension) but not the former. So instead of > just one "dump" flag, we need two such flags for namespaces. The > various selectDumpableFoo routines need one-line adjustments as to > which flag they look at in the parent namespace. They do *not* > need copy-and-paste of their entire current contents. Agreed. > In HEAD, this should be a pretty straightforward change since Stephen > Frost has already created infrastructure that allows distinguishing which > "components" of an object to dump. There doesn't seem to be a bit that > corresponds to "objects within this schema", but that shouldn't be hard > to add. > The DUMP_COMPONENT infrastructure doesn't exist in older branches > so I think you'd just have to invent an additional bool field in > NamespaceInfo structs to say whether to dump contained objects. In HEAD, this already exists in the form of 'dump_contains', which sits next to 'dump' in the DumpableObject struct. I added that independent field as it allows us to say things like "dump components X, Y, Z of this object, and components A, B, C of objects *contained* by this object." What that means is that we can dump, say, just the privileges of a namespace, but dump all of the components of objects which live inside of that namespace. If all we need is a flag for the back branches (I've not looked very closely yet...), then I'd suggest simply adding that 'dump_contains' field as a boolean in the back branches and using that. > A slightly different approach is to ignore the DumpComponents stuff > and just add a bool field in NamespaceInfo for all branches. Perhaps > this would even be the right thing, as I may be misunderstanding what > Stephen intends the DumpComponents bits to be used for. Would be > interesting to hear Stephen's take on this. Putting 'dump_contains' into the NamespaceInfo wouldn't work as objects which are members of extensions are also "contained" by that extension, allowing us to say things like "dump all of the ACLs for all of the objects which are members of this extension, but not the extension itself." We could possibly rework it such that all 'dump_contains' bitmasks are part of the data structures for the 'containing' object instead of being associated with *every* object. That might be a bit cleaner, but might also mean that it gets forgotten as it's not sitting right next to the main 'dump' bitmask. Thanks! Stephen
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> In HEAD, this should be a pretty straightforward change since Stephen
>> Frost has already created infrastructure that allows distinguishing which
>> "components" of an object to dump. There doesn't seem to be a bit that
>> corresponds to "objects within this schema", but that shouldn't be hard
>> to add.
> In HEAD, this already exists in the form of 'dump_contains', which sits
> next to 'dump' in the DumpableObject struct. I added that independent
> field as it allows us to say things like "dump components X, Y, Z of
> this object, and components A, B, C of objects *contained* by this
> object." What that means is that we can dump, say, just the privileges
> of a namespace, but dump all of the components of objects which live
> inside of that namespace.
Ah. So maybe we just need to rearrange selectDumpableNamespace.
Offhand, it seems like we want checkExtensionMembership to adjust the
namespace's dump bits, but not its dump_contained bits (which indeed it
already doesn't, which until just now I would have said is an oversight).
So maybe just moving the checkExtensionMembership call to after the other
stuff would be enough in HEAD?
regards, tom lane
El 22/06/16 a las 12:12, Tom Lane escribió: > > Ah. So maybe we just need to rearrange selectDumpableNamespace. > Offhand, it seems like we want checkExtensionMembership to adjust the > namespace's dump bits, but not its dump_contained bits (which indeed it > already doesn't, which until just now I would have said is an oversight). On HEAD I actually see that we *are* setting dump_contains as well: if (fout->remoteVersion < 90600)dobj->dump = DUMP_COMPONENT_NONE; elsedobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL | DUMP_COMPONENT_SECLABEL | DUMP_COMPONENT_POLICY); > So maybe just moving the checkExtensionMembership call to after the other > stuff would be enough in HEAD? 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) If a non extension table belongs to a schema created by an extension, the schema will have nsinfo->dobj.dump = false, so when we check if the non extension table is dumpable we end up with this (unless there's -t or -T which is not interesting for this patch): tbinfo->dobj.dump = tbinfo->dobj.namespace->dobj.dump; On HEAD (where I started looking at this bug) the situation is similar. It might just be that I'm missing something here. Regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Martín Marqués <martin@2ndquadrant.com> writes:
> On HEAD I actually see that we *are* setting dump_contains as well:
> if (fout->remoteVersion < 90600)
> dobj->dump = DUMP_COMPONENT_NONE;
> else
> dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL |
> DUMP_COMPONENT_SECLABEL | DUMP_COMPONENT_POLICY);
Uh, what?
(BTW, Stephen, what's the point of the version check here? The 'else'
code seems like it would work just fine in all branches.)
>> So maybe just moving the checkExtensionMembership call to after the other
>> stuff would be enough in HEAD?
> 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)
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.
regards, tom lane
El 22/06/16 a las 14:21, Tom Lane escribió: > >> 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) > > 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. Oh, that's very reasonable. I wonder how that would look on version < 9.6. Will have a look at it on HEAD and 9.5 (I guess changes back to 9.2 or 9.1 should be fairly similar, but I'll verify) -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
* 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
Hi, couldn't find a committed patch to fix this. Did I miss it in the pile of logs? El 22/06/16 a las 14:21, Tom Lane escribió: > >>> So maybe just moving the checkExtensionMembership call to after the other >>> stuff would be enough in HEAD? > >> 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) > > 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. Was drifted away but can make some time now to add a proper test for this and a fix around Tom's thoughts. Regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Aug 10, 2016 at 4:27 AM, Mart=C3=ADn Marqu=C3=A9s <martin@2ndquadra= nt.com> wrote: > El 22/06/16 a las 14:21, Tom Lane escribi=C3=B3: >>>> So maybe just moving the checkExtensionMembership call to after the ot= her >>>> stuff would be enough in HEAD? >> >>> 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) >> >> 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. > Hi, couldn't find a committed patch to fix this. Did I miss it in the > pile of logs? No, it seems that everybody was waiting for you here, and least that's my c= ase. > Was drifted away but can make some time now to add a proper test for > this and a fix around Tom's thoughts. Yes, that would be nice. And some necessary tests can be added at least on HEAD, see for example f9e439b1. --=20 Michael
Hi Michael, 2016-08-09 20:25 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>: > On Wed, Aug 10, 2016 at 4:27 AM, Martín Marqués <martin@2ndquadrant.com> wrote: > >> Hi, couldn't find a committed patch to fix this. Did I miss it in the >> pile of logs? > > No, it seems that everybody was waiting for you here, and least that's my case. Yeah, I thought so, but wanted to be sure before spending time on this. >> Was drifted away but can make some time now to add a proper test for >> this and a fix around Tom's thoughts. > > Yes, that would be nice. And some necessary tests can be added at > least on HEAD, see for example f9e439b1. I actually started with the tests, as it makes it easier to know if the changes don't break anything. ;) Yesterday I was investigating (for the first time) how TPA is implemented in postgres (I actually focused on the pg_dump test in the modules folder) and added a schema with a table in to the dummy extension we use for testing in src/test/modules/test_pg_dump/test_pg_dump--1.0.sql. I'll have a look at that commit and see if I can the test's in a patch and from there apply the suggestion from Tom. regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services