Re: pg_dump with tables created in schemas created by extensions

On Wed, Aug 24, 2016 at 9:07 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Aug 24, 2016 at 5:34 AM, Martín Marqués <martin@2ndquadrant.com> wrote:
>> Hi,
>>
>> 2016-08-23 16:46 GMT-03:00 Martín Marqués <martin@2ndquadrant.com>:
>>>
>>> I will add tests for sequence and functions as you mention and test again.
>>>
>>> Then I'll check if other tests should be added as well.
>>
>> I found quite some other objects we should be checking as well, but
>> this will add some duplication to the tests, as I'd just copy (with
>> minor changes) what's in src/bin/pg_dump/t/002_pg_dump.pl
>>
>> I can't think of a way to avoid this duplication, not that it really
>> hurts. We would have to make sure that any new objects added to one
>> test, if needed, are added to the other (that's a bit cumbersome).
>>
>> Other things to check:
>>
>> CREATE AGGREGATE
>> CREATE FUNCTION

Agreed on those two.

>> CREATE DOMAIN
>> CREATE TYPE

Those two overlap, so adding just a type would be fine.

>> CREATE MATERIALIZED VIEW

This overlaps with normal relations.

>> CREATE POLICY

Policies are not schema-qualified.

>> Maybe even CREATE INDEX over a table created in the schema.

Yes, we can do something fancy things here... But see below as pg_dump
is broken even in this case...

>> Also, ACLs have to be tested against objects in the schema.
>>
>> I hope I didn't miss anything there.

So I have reviewed the patch for master, did some wordsmithing and
added more tests. Particularly, I have added tests for a couple of
objects created in the extension, as well as objects that are not part
of the extension but make use of the schema of the extension. Even
with that, I am still seeing two problems:
- ACLs assigned to an aggregate in an extension are not getting dumped
in a binary upgrade, and I think that they should be. The aggregate
definition gets correctly handled though.
- if an index is defined on a table part of an extension it will not
be dumped. We can fix the problem of this thread and the one I just
found separately though.
The patch attached includes all those tests and they are failing. We
are going to need a patch able to pass all that, and even for master
this is going to need more thoughts, and let's focus on HEAD/9.6
first.
--
Michael