|From:||Stephen Frost <sfrost(at)snowman(dot)net>|
|To:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||Daniele Varrazzo <daniele(dot)varrazzo(at)gmail(dot)com>, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org>|
|Subject:||Re: pg_dump 9.6 doesn't honour pg_extension_config_dump for sequences|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
* Michael Paquier (michael(dot)paquier(at)gmail(dot)com) wrote:
> On Thu, Jan 19, 2017 at 10:46 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > Yeah, that looks like the right answer and matches how dumpTableData
> > handles this, which is also why that's working correctly.
> > I don't particularly like this arrangement though. This works because:
> > getTables() will create the entry for the sequence and call
> > selectDumpableTable(), which calls checkExtensionMembership(), which
> > will set the dump flags to ACL | SECLABEL | POLICY, and then
> > getTableData() will skip adding a data entry for it because DATA isn't
> > set, allowing processExtensionTables() to add the DATA entry later for
> > config tables/sequences.
> Yes, I certainly agree with that. When looking at this bug the first
> thing I did was to set DUMP_COMPONENT_DATA in
> checkExtensionMembership() this way actually to see that nothing
Nothing happened..? getTableData() should have then created an entry
for it and then dump it out.. If that didn't happen then I'm kind of
curious as to why not.
> It would be also better to have a sanity check like
> (tbinfo->obj.dataObj != NULL && (tbinfo->dump & DUMP_COMPONENT_DATA)
> != 0) in both the sequence and table data dump paths, and use tbinfo
> instead of tdinfo to do this work. This way we check the dependency
> between the DATA flag set and the created dataObj by
Well, yes, if we go the route that I suggested and make the tbinfo
actually have the DATA component set for these objects by
checkExtensionMembership(). We can't set that kind of a sanity check
now, of course.
> Thanks. There are still no tests for dumpable tables though.
Agreed. I have a whole slew of additional regression tests slated for
the src/bin/pg_dump/t/ set of tests, I wouldn't complain if someone
wanted to work through adding more regression tests to
src/test/modules/test_pg_dump/t/ for extensions...
Otherwise, I'll get there once I've gotten the code coverage of pg_dump
*without* worrying about extensions up to something more reasonable.
Currently there are far too many cases that aren't covered (as evidenced
by the current rate of bug-fix back-patching that I'm doing..).
|Next Message||Kyotaro HORIGUCHI||2017-01-20 02:07:29||Re: Bug in Physical Replication Slots (at least 9.5)?|
|Previous Message||Michael Paquier||2017-01-20 01:25:50||Re: pg_dump 9.6 doesn't honour pg_extension_config_dump for sequences|