Fix dependency recording bug for partitioned PKs

When DefineIndex recurses to create constraints on partitions, it needs
to use the value returned by index_constraint_create to set up partition
dependencies.  However, in the course of fixing the DEPENDENCY_INTERNAL_AUTO
mess, commit 1d92a0c9f7dd introduced some code to that function that
clobbered the return value, causing the recorded OID to be of the wrong
object.  Close examination of pg_depend after creating the tables leads
to indescribable objects :-( My sin (in commit bdc3d7fa2376, while
preparing for DDL deparsing in event triggers) was to use a variable
name for the return value that's typically used for throwaway objects in
dependency-setting calls ("referenced").  Fix by changing the variable
names to match extended practice (the return value is "myself" rather
than "referenced".)

The pg_upgrade test notices the problem (in an indirect way: the pg_dump
outputs are in different order), but only if you create the objects in a
specific way that wasn't being used in the existing tests.  Add a stanza
to leave some objects around that shows the bug.

Catversion bump because preexisting databases might have bogus pg_depend
entries.

Discussion: https://postgr.es/m/20190318204235.GA30360@alvherre.pgsql

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index c339a2bb779c3aa26b0377f957a2e7e11827cc30..cb2c0010171cc0a3dce1c107a38e02f90afbdf41 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1247,7 +1247,7 @@ index_constraint_create(Relation heapRelation,
 {
        Oid                     namespaceId = RelationGetNamespace(heapRelation);
        ObjectAddress myself,
-                               referenced;
+                               idxaddr;
        Oid                     conOid;
        bool            deferrable;
        bool            initdeferred;
@@ -1341,15 +1341,9 @@ index_constraint_create(Relation heapRelation,
         * Note that the constraint has a dependency on the table, so we don't
         * need (or want) any direct dependency from the index to the table.
         */
-       myself.classId = RelationRelationId;
-       myself.objectId = indexRelationId;
-       myself.objectSubId = 0;
-
-       referenced.classId = ConstraintRelationId;
-       referenced.objectId = conOid;
-       referenced.objectSubId = 0;
-
-       recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
+       ObjectAddressSet(myself, ConstraintRelationId, conOid);
+       ObjectAddressSet(idxaddr, RelationRelationId, indexRelationId);
+       recordDependencyOn(&idxaddr, &myself, DEPENDENCY_INTERNAL);
 
        /*
         * Also, if this is a constraint on a partition, give it partition-type
@@ -1357,7 +1351,8 @@ index_constraint_create(Relation heapRelation,
         */
        if (OidIsValid(parentConstraintId))
        {
-               ObjectAddressSet(myself, ConstraintRelationId, conOid);
+               ObjectAddress   referenced;
+
                ObjectAddressSet(referenced, ConstraintRelationId, parentConstraintId);
                recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_PRI);
                ObjectAddressSet(referenced, RelationRelationId,
@@ -1444,7 +1439,7 @@ index_constraint_create(Relation heapRelation,
                table_close(pg_index, RowExclusiveLock);
        }
 
-       return referenced;
+       return myself;
 }
 
 /*

diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 1f74cc4d4486cbf78d4c382232f9f54680fe332c..8f25d71ba128c89d525997a7a15c539291b358f2 100644 (file)
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1411,6 +1411,24 @@ alter index idxpart2_a_idx attach partition idxpart22_a_idx;
 create index on idxpart (a);
 create table idxpart_another (a int, b int, primary key (a, b)) partition by range (a);
 create table idxpart_another_1 partition of idxpart_another for values from (0) to (100);
+-- More objects intentionally left behind, to verify some pg_dump/pg_upgrade
+-- behavior; see https://postgr.es/m/20190321204928.GA17535@alvherre.pgsql
+create schema regress_indexing;
+set search_path to regress_indexing;
+create table pk (a int primary key) partition by range (a);
+create table pk1 partition of pk for values from (0) to (1000);
+create table pk2 (b int, a int);
+alter table pk2 drop column b;
+alter table pk2 alter a set not null;
+alter table pk attach partition pk2 for values from (1000) to (2000);
+create table pk3 partition of pk for values from (2000) to (3000);
+create table pk4 (like pk);
+alter table pk attach partition pk4 for values from (3000) to (4000);
+create table pk5 (like pk) partition by range (a);
+create table pk51 partition of pk5 for values from (4000) to (4500);
+create table pk52 partition of pk5 for values from (4500) to (5000);
+alter table pk attach partition pk5 for values from (4000) to (5000);
+reset search_path;
 -- Test that covering partitioned indexes work in various cases
 create table covidxpart (a int, b int) partition by list (a);
 create unique index on covidxpart (a) include (b);

diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 42b4b03b817d65bd7e2bf5e2849c9abb093c7b1b..2ccea02cae0a8feea54f9ab11a4ba470f9253dc7 100644 (file)
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -748,6 +748,25 @@ create index on idxpart (a);
 create table idxpart_another (a int, b int, primary key (a, b)) partition by range (a);
 create table idxpart_another_1 partition of idxpart_another for values from (0) to (100);
 
+-- More objects intentionally left behind, to verify some pg_dump/pg_upgrade
+-- behavior; see https://postgr.es/m/20190321204928.GA17535@alvherre.pgsql
+create schema regress_indexing;
+set search_path to regress_indexing;
+create table pk (a int primary key) partition by range (a);
+create table pk1 partition of pk for values from (0) to (1000);
+create table pk2 (b int, a int);
+alter table pk2 drop column b;
+alter table pk2 alter a set not null;
+alter table pk attach partition pk2 for values from (1000) to (2000);
+create table pk3 partition of pk for values from (2000) to (3000);
+create table pk4 (like pk);
+alter table pk attach partition pk4 for values from (3000) to (4000);
+create table pk5 (like pk) partition by range (a);
+create table pk51 partition of pk5 for values from (4000) to (4500);
+create table pk52 partition of pk5 for values from (4500) to (5000);
+alter table pk attach partition pk5 for values from (4000) to (5000);
+reset search_path;
+
 -- Test that covering partitioned indexes work in various cases
 create table covidxpart (a int, b int) partition by list (a);
 create unique index on covidxpart (a) include (b);