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 1d92a0c9f7 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 bdc3d7fa23, 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
This commit is contained in:
Alvaro Herrera 2019-03-21 18:34:29 -03:00
parent bfb456c1b9
commit 7e7c57bbb2
3 changed files with 44 additions and 12 deletions

View File

@ -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;
}
/*

View File

@ -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);

View File

@ -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);