pg_dump with tables created in schemas created by extensions

On Sat, Aug 13, 2016 at 6:58 AM, Martín Marqués <martin@2ndquadrant.com> wrote:

I believe the fix will be simple after the back and forth mails with
Michael, Stephen and Tom. I will work on that later, but preferred to
have the tests the show the problem which will also make testing the fix
easier.

Thoughts?

It seems to me that we are going to need a bit more coverage for more
object types depending on the code paths that are being changed. For
example, sequences or functions should be checked for as well, and not
only tables. By the way, do you need help to build a patch or should I
jump in?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Michael,

2016-08-23 5:02 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>:

On Sat, Aug 13, 2016 at 6:58 AM, Martín Marqués <martin@2ndquadrant.com> wrote:

I believe the fix will be simple after the back and forth mails with
Michael, Stephen and Tom. I will work on that later, but preferred to
have the tests the show the problem which will also make testing the fix
easier.

Thoughts?

It seems to me that we are going to need a bit more coverage for more
object types depending on the code paths that are being changed. For
example, sequences or functions should be checked for as well, and not
only tables. By the way, do you need help to build a patch or should I
jump in?

I wanted to test what I had in mind with one object, and then see if
any replication is needed for other objects.

I was struggling the last days as what I was reading in my patched
pg_dump.c had to work as expected, and dump the tables not created by
the test_pg_dump extension but inside the schema
regress_pg_dump_schema.

Today I decided to go over the test I wrote, and found a bug there,
reason why I couldn't get a successful make check.

Here go 2 patches. One is a fix for the test I sent earlier. The other
is the proposed idea Tom had using the dump_contains that Stephan
committed on 9.6.

So far I've checked that it fixes the dumpable for Tables, but I think
it should work for all other objects as well, as all this patch does
is leave execution of checkExtensionMembership at the end of
selectDumpableNamespace, leaving the dump_contains untouched.

Checks pass ok.

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.

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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 DOMAIN
CREATE FUNCTION
CREATE TYPE
CREATE MATERIALIZED VIEW
CREATE POLICY

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

Also, ACLs have to be tested against objects in the schema.

I hope I didn't miss anything there.

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/23/16 3:34 PM, Martín Marqués wrote:

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).

At one point I had some code that understood what object names (ie:
AGGREGATE, TABLE, etc) went with what catalog tables, what ones lived
inside a schema (as opposed to globally), and which ones were
shared/global (cross-database). I think I needed this for some automatic
handling of comments, but it's been a while. Maybe something like that
would help reduce the duplication...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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 DOMAIN
CREATE FUNCTION
CREATE TYPE
CREATE MATERIALIZED VIEW
CREATE POLICY

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

Also, ACLs have to be tested against objects in the schema.

I hope I didn't miss anything there.

I'll take a look at that soon and review your patch as well as your
tests. For now I have added an entry in the CF app to not lose track
of it:
https://commitfest.postgresql.org/10/736/
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

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.

Are you sure you have the tests correct..? At least for testagg(), it
looks like you're testing for:

GRANT ALL ON FUNCTION test_agg(int2) TO regress_dump_test_role;

but what's in the dump is (equivilantly):

GRANT ALL ON FUNCTION test_agg(smallint) TO regress_dump_test_role;

I've not looked into all the failures, but at least this one seems like
an issue in the test, not an issue in pg_dump.

Thanks!

Stephen

2016-08-24 11:15 GMT-03:00 Stephen Frost <sfrost@snowman.net>:

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

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.

Are you sure you have the tests correct..? At least for testagg(), it
looks like you're testing for:

GRANT ALL ON FUNCTION test_agg(int2) TO regress_dump_test_role;

but what's in the dump is (equivilantly):

GRANT ALL ON FUNCTION test_agg(smallint) TO regress_dump_test_role;

Yes, that was the problem there.

I've not looked into all the failures, but at least this one seems like
an issue in the test, not an issue in pg_dump.

I see the other 12 failures regarding the CREATE INDEX that Michael
reported but can't quite find where it's originated. (or actually
where the problem is)

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2016-08-24 17:01 GMT-03:00 Martín Marqués <martin@2ndquadrant.com>:

2016-08-24 11:15 GMT-03:00 Stephen Frost <sfrost@snowman.net>:

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

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.

Are you sure you have the tests correct..? At least for testagg(), it
looks like you're testing for:

GRANT ALL ON FUNCTION test_agg(int2) TO regress_dump_test_role;

but what's in the dump is (equivilantly):

GRANT ALL ON FUNCTION test_agg(smallint) TO regress_dump_test_role;

Yes, that was the problem there.

I've not looked into all the failures, but at least this one seems like
an issue in the test, not an issue in pg_dump.

I see the other 12 failures regarding the CREATE INDEX that Michael
reported but can't quite find where it's originated. (or actually
where the problem is)

OK, I see where the problem is.

Indexes don't have a selectDumpableIndex() function to see if we dump
it or not. We just don't gather indexes from tables for which we are
dumping their definition:

/* Ignore indexes of tables whose definitions are not
to be dumped */
if (!(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
continue;

This means we have to perform the same change we did on
selectDumpableNamespace for selectDumpableTable, and also assign the
correct value to dump_contains, which is not set there.

The problem will come when we have to decide on which indexes were
created by the extension (primary key indexes, other indexes created
by the extension) and which were created afterwards over a table which
depends on the extension (the test_table from the extension).

Right now, I'm in an intermediate state, where I got getIndexes() to
query for the indexes of these tables, but dumpIndexes is not dumping
the indexes that were queried.

I wonder if we should have a selectDumpableIndexes to set the
appropriate dobj.dump for the Indexes.

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Aug 24, 2016 at 11:15 PM, Stephen Frost <sfrost@snowman.net> wrote:

* Michael Paquier (michael.paquier@gmail.com) wrote:

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.

Are you sure you have the tests correct..? At least for testagg(), it
looks like you're testing for:

GRANT ALL ON FUNCTION test_agg(int2) TO regress_dump_test_role;

but what's in the dump is (equivilantly):

GRANT ALL ON FUNCTION test_agg(smallint) TO regress_dump_test_role;

I've not looked into all the failures, but at least this one seems like
an issue in the test, not an issue in pg_dump.

Yes, you are right. If I look at the diffs this morning I am seeing
the ACLs being dumped for this aggregate. So we could just fix the
test and be done with it. I did not imagine the index issue though,
and this is broken for some time, so that's not exclusive to 9.6 :)
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2016-08-24 21:34 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>:

Yes, you are right. If I look at the diffs this morning I am seeing
the ACLs being dumped for this aggregate. So we could just fix the
test and be done with it. I did not imagine the index issue though,
and this is broken for some time, so that's not exclusive to 9.6 :)

Hi Michael,

Do you see any easier way than what I mentioned earlier (adding a
selectDumpableIndex() function) to fix the index dumping issue?

Regards,

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Aug 25, 2016 at 10:25 AM, Martín Marqués <martin@2ndquadrant.com> wrote:

2016-08-24 21:34 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>:

Yes, you are right. If I look at the diffs this morning I am seeing
the ACLs being dumped for this aggregate. So we could just fix the
test and be done with it. I did not imagine the index issue though,
and this is broken for some time, so that's not exclusive to 9.6 :)

Do you see any easier way than what I mentioned earlier (adding a
selectDumpableIndex() function) to fix the index dumping issue?

Yes, we are going to need something across those lines. And my guess
is that this is going to be rather close to getOwnedSeqs() in terms of
logic.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

2016-08-25 8:10 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>:

On Thu, Aug 25, 2016 at 10:25 AM, Martín Marqués <martin@2ndquadrant.com> wrote:

2016-08-24 21:34 GMT-03:00 Michael Paquier <michael.paquier@gmail.com>:

Yes, you are right. If I look at the diffs this morning I am seeing
the ACLs being dumped for this aggregate. So we could just fix the
test and be done with it. I did not imagine the index issue though,
and this is broken for some time, so that's not exclusive to 9.6 :)

Do you see any easier way than what I mentioned earlier (adding a
selectDumpableIndex() function) to fix the index dumping issue?

Yes, we are going to need something across those lines. And my guess
is that this is going to be rather close to getOwnedSeqs() in terms of
logic.

I was able to get this fixed without any further new functions (just
using the dump/dump_contains and applying the same fix on
selectDumpableTable).

Main problem relied here in getIndexes()

@@ -6158,7 +6167,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[],
int numTables)
continue;

                /* Ignore indexes of tables whose definitions are not
to be dumped */
-               if (!(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
+               if (!(tbinfo->dobj.dump_contains & DUMP_COMPONENT_DEFINITION))
                        continue;

if (g_verbose)

But we have to set dump_contains with correct values.

There's still one issue, which I'll add a test for as well, which is
that if the index was created by the extension, it will be dumped
anyway. I'll ave a look at that as well.

One other thing I found was that one of the CREATE INDEX tests had
incorrectly set like and unlike for pre_data and post_data. (indexes
are dumped in section post_data)

That's been fixes as well.

I've cleaned up the patch a bit, so this is v3 with all checks passing.

I'll add that new test regarding dumping an index created by the
extension (which will fail) and look for ways to fix it.

Regards,

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hi,

2016-08-26 10:53 GMT-03:00 Martín Marqués <martin@2ndquadrant.com>:

There's still one issue, which I'll add a test for as well, which is
that if the index was created by the extension, it will be dumped
anyway. I'll have a look at that as well.

Looking at this issue today, I found that we are not setting a
dependency for an index created inside an extension. I don't know if
it's deliberate or an overlook.

The thing is that we can't check pg_depend for dependency of an index
and the extension that creates it.

I was talking with other developers, and we kind of agree this is a
bug, for 2 reasons we thought of:

*) If the extension only creates an index over an existing table, a
drop extension will not drop that index

*) We need to have the dependency for this patch as well, else we'll
end up with an inconsistent dump, or at least one that could restore
with a != 0 return error code.

I'll open a separate bug report for this.

Regards,

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

=?UTF-8?B?TWFydMOtbiBNYXJxdcOpcw==?= <martin@2ndquadrant.com> writes:

Looking at this issue today, I found that we are not setting a
dependency for an index created inside an extension.

Surely the index has a dependency on a table, which depends on the
extension?

If you mean that you want an extension to create an index on a table that
doesn't belong to it, but it's assuming pre-exists, I think that's just
stupid and we need not support it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/27/2016 12:37 AM, Tom Lane wrote:

=?UTF-8?B?TWFydMOtbiBNYXJxdcOpcw==?= <martin@2ndquadrant.com> writes:

Looking at this issue today, I found that we are not setting a
dependency for an index created inside an extension.

Surely the index has a dependency on a table, which depends on the
extension?

If you mean that you want an extension to create an index on a table
that doesn't belong to it, but it's assuming pre-exists, I think
that's just stupid and we need not support it.

I don't see why that would be stupid (and I'm not sure it's up to us to
just decide it's stupid).

Imagine you use extensions to break the application into modules. You
have a "basic" extension, with the table, and a "search" extension
implementing fulltext search on top of that table. Isn't it natural to
keep the gin indexes in the search extension?

So the basic--1.0.sql will do something like

CREATE TABLE x ( ...)

and search--1.0.sql would do

CREATE INDEX y ON x USING gin (z);
CREATE FUNCTION search_objects(..) ...

because the index and function doing the search are somewhat tightly
coupled. I admit this is just an example and I don't know how many
people use extensions this way, but I don't see why this would be
inherently stupid pattern?

I see the current behavior is documented, and I do understand why global
objects can't be part of the extension, but for indexes it seems to
violate POLA a bit.

Is there a reason why we don't want the extension/index dependencies?

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:

On 08/27/2016 12:37 AM, Tom Lane wrote:

If you mean that you want an extension to create an index on a table
that doesn't belong to it, but it's assuming pre-exists, I think
that's just stupid and we need not support it.

I don't see why that would be stupid (and I'm not sure it's up to us to
just decide it's stupid).

Well, think about it.

1. Let's say user A owns the pre-existing table and user B owns the
extension. Who owns the index?

2. Generally speaking, if an object is part of an extension then you
can't drop the object without dropping the whole extension. This means
that either user A can't drop his own table, or he can but that causes
dropping the whole extension (which he does not own), not to mention
whatever else depends on it (which he owns even less).

3. Can user A do something that would mutate the index? (For instance,
ALTER COLUMN TYPE on one of the columns it's on.) Now is it still a
member of the extension? If user A can't, can user B? What if either of
them mutated the column to a datatype that belongs to some extension that
has a dependency on the original one? Now you've got a circular
dependency loop, what fun --- especially at dump/reload time, where
pg_dump has no hope of breaking the circularity.

4. If we're going to support indexes as independent members of extensions,
why not foreign-key constraints, or check constraints? Maybe check
constraints on domain types that don't belong to the extension? Heck,
why not allow an extension to do ALTER TABLE ADD COLUMN? (I think
BTW that several of these cases would allow creation of circular
extension dependencies even without any ALTER COLUMN TYPE shenanigans.)

None of this makes any sense, because these things are not stand-alone
objects in any sense of the word. They are attributes of tables (or
domain types, for that case) and there are good reasons why we don't
consider such attributes to have ownership independent of the object
they are part of. I do not think it is sensible for an extension to "own"
objects that don't also have, or could potentially have [1]weasel wording for cases like casts, which we don't consider to have owners, though I'm not sure why not., independent
ownership in the sense of an owning user.

Imagine you use extensions to break the application into modules.

I do not think that extensions as we currently understand them are a
suitable basis for slicing an application in the way you suggest.
I'm fine with an extension owning a whole table, but the rest of this
is just crazy. And I sure as hell do not want to put it in as part
of a bug fix for existing releases.

regards, tom lane

[1]: weasel wording for cases like casts, which we don't consider to have owners, though I'm not sure why not.
to have owners, though I'm not sure why not.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/26/2016 04:15 PM, Tomas Vondra wrote:

On 08/27/2016 12:37 AM, Tom Lane wrote:

=?UTF-8?B?TWFydMOtbiBNYXJxdcOpcw==?= <martin@2ndquadrant.com> writes:

Looking at this issue today, I found that we are not setting a
dependency for an index created inside an extension.

Surely the index has a dependency on a table, which depends on the
extension?

If you mean that you want an extension to create an index on a table
that doesn't belong to it, but it's assuming pre-exists, I think
that's just stupid and we need not support it.

I don't see why that would be stupid (and I'm not sure it's up to us to
just decide it's stupid).

+1, there are a lot of workflow patterns that make sense and don't make
sense depending on your perspective, consider safe mode with MySQL. I
personally think it is stupid too but it also obviously has a huge
useful user base.

Imagine you use extensions to break the application into modules. You
have a "basic" extension, with the table, and a "search" extension
implementing fulltext search on top of that table. Isn't it natural to
keep the gin indexes in the search extension?

So the basic--1.0.sql will do something like

CREATE TABLE x ( ...)

and search--1.0.sql would do

CREATE INDEX y ON x USING gin (z);
CREATE FUNCTION search_objects(..) ...

because the index and function doing the search are somewhat tightly
coupled. I admit this is just an example and I don't know how many
people use extensions this way, but I don't see why this would be
inherently stupid pattern?

It isn't and in fact shows that as we extend Pg, the more we allow
people flexibility in developing WITHIN the database, the more we will
be able to influence them to do so. That is a good thing.

Or maybe we should just tell everyone, use an ORM it will solve all your
problems. (/sarcasm)

Sincerely,

JD

--
Command Prompt, Inc. http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2016-08-26 19:37 GMT-03:00 Tom Lane <tgl@sss.pgh.pa.us>:

=?UTF-8?B?TWFydMOtbiBNYXJxdcOpcw==?= <martin@2ndquadrant.com> writes:

Looking at this issue today, I found that we are not setting a
dependency for an index created inside an extension.

Surely the index has a dependency on a table, which depends on the
extension?

If you mean that you want an extension to create an index on a table that
doesn't belong to it, but it's assuming pre-exists, I think that's just
stupid and we need not support it.

Well, there's still the second pattern I mentioned before (which
actually came up while trying to fix this patch).

Extension creates a table and an index over one of the columns:

CREATE TABLE regress_pg_dump_schema.test_table (
col1 int,
col2 int
);

CREATE INDEX test_extension_index ON regress_pg_dump_schema.test_table (col2);

Later, some application (or a user, doesn't matter really) creates a
second index over col1:

CREATE INDEX test_index ON regress_pg_dump_schema.test_table (col1);

What we are doing (or at least it's what I understand from the code)
is checking if the table depends on an extension, and so we don't dump
it.

We should be able to use the same procedure (and reuse the code we
already have) to decide if an index should be dumped or not. But we
are missing the dependency, and so it's not possible to know that
regress_pg_dump_schema.test_extension_index depends on the extension
and regress_pg_dump_schema.test_index doesn't.

Or is this something we shouldn't support (in that case we should document it).

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers