From 7e7c57bbb2ebed7e8acbd2e62fadca5a5fe5df5f Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 21 Mar 2019 18:34:29 -0300 Subject: [PATCH] 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 --- src/backend/catalog/index.c | 19 +++++++------------ src/test/regress/expected/indexing.out | 18 ++++++++++++++++++ src/test/regress/sql/indexing.sql | 19 +++++++++++++++++++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index c339a2bb77..cb2c001017 100644 --- 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 1f74cc4d44..8f25d71ba1 100644 --- 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 42b4b03b81..2ccea02cae 100644 --- 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);