Fix interference between covering indexes and partitioned tables

The bug is caused due to the original IndexStmt that DefineIndex receives
being overwritten when processing the INCLUDE columns. Use separate list of
index params to propagate to child tables. Add tests covering this case.

Amit Langote and Alexander Korotkov.

Re-commit 5c6110c6a960ad6fe1b0d0fec6ae36ef4eb913f5 because it discovered a bug
fixed in c266ed31a8a3beed3533e6a78faeca78234cbd43

Discussion: https://www.postgresql.org/message-id/CAJGNTeO%3DBguEyG8wxMpU_Vgvg3nGGzy71zUQ0RpzEn_mb0bSWA%40mail.gmail.com

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 61a4b24437b143b859964a7cd659615e4dd03838..78302544db852c06f9e563d624ca5e3fa865836b 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -342,6 +342,7 @@ DefineIndex(Oid relationId,
        Oid                     tablespaceId;
        Oid                     createdConstraintId = InvalidOid;
        List       *indexColNames;
+       List       *allIndexParams;
        Relation        rel;
        Relation        indexRelation;
        HeapTuple       tuple;
@@ -378,16 +379,16 @@ DefineIndex(Oid relationId,
        numberOfKeyAttributes = list_length(stmt->indexParams);
 
        /*
-        * We append any INCLUDE columns onto the indexParams list so that we have
-        * one list with all columns.  Later we can determine which of these are
-        * key columns, and which are just part of the INCLUDE list by checking
-        * the list position.  A list item in a position less than
-        * ii_NumIndexKeyAttrs is part of the key columns, and anything equal to
-        * and over is part of the INCLUDE columns.
+        * Calculate the new list of index columns including both key columns and
+        * INCLUDE columns.  Later we can determine which of these are key columns,
+        * and which are just part of the INCLUDE list by checking the list
+        * position.  A list item in a position less than ii_NumIndexKeyAttrs is
+        * part of the key columns, and anything equal to and over is part of the
+        * INCLUDE columns.
         */
-       stmt->indexParams = list_concat(stmt->indexParams,
-                                                                       stmt->indexIncludingParams);
-       numberOfAttributes = list_length(stmt->indexParams);
+       allIndexParams = list_concat(list_copy(stmt->indexParams),
+                                                                list_copy(stmt->indexIncludingParams));
+       numberOfAttributes = list_length(allIndexParams);
 
        if (numberOfAttributes <= 0)
                ereport(ERROR,
@@ -544,7 +545,7 @@ DefineIndex(Oid relationId,
        /*
         * Choose the index column names.
         */
-       indexColNames = ChooseIndexColumnNames(stmt->indexParams);
+       indexColNames = ChooseIndexColumnNames(allIndexParams);
 
        /*
         * Select name for index if caller didn't specify
@@ -658,7 +659,7 @@ DefineIndex(Oid relationId,
        coloptions = (int16 *) palloc(numberOfAttributes * sizeof(int16));
        ComputeIndexAttrs(indexInfo,
                                          typeObjectId, collationObjectId, classObjectId,
-                                         coloptions, stmt->indexParams,
+                                         coloptions, allIndexParams,
                                          stmt->excludeOpNames, relationId,
                                          accessMethodName, accessMethodId,
                                          amcanorder, stmt->isconstraint);
@@ -886,8 +887,8 @@ DefineIndex(Oid relationId,
                        memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
 
                        parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));
-                       opfamOids = palloc(sizeof(Oid) * numberOfAttributes);
-                       for (i = 0; i < numberOfAttributes; i++)
+                       opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
+                       for (i = 0; i < numberOfKeyAttributes; i++)
                                opfamOids[i] = get_opclass_family(classObjectId[i]);
 
                        heap_close(rel, NoLock);

diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 33f68aab7123a5ec12868e337c32b4a4f5fb6e0f..2c2bf44aa87d823fe73fd23dcd8814256392ba50 100644 (file)
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1313,3 +1313,27 @@ 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);
+-- 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);
+create table covidxpart1 partition of covidxpart for values in (1);
+create table covidxpart2 partition of covidxpart for values in (2);
+insert into covidxpart values (1, 1);
+insert into covidxpart values (1, 1);
+ERROR:  duplicate key value violates unique constraint "covidxpart1_a_b_idx"
+DETAIL:  Key (a)=(1) already exists.
+create table covidxpart3 (b int, c int, a int);
+alter table covidxpart3 drop c;
+alter table covidxpart attach partition covidxpart3 for values in (3);
+insert into covidxpart values (3, 1);
+insert into covidxpart values (3, 1);
+ERROR:  duplicate key value violates unique constraint "covidxpart3_a_b_idx"
+DETAIL:  Key (a)=(3) already exists.
+create table covidxpart4 (b int, a int);
+create unique index on covidxpart4 (a) include (b);
+create unique index on covidxpart4 (a);
+alter table covidxpart attach partition covidxpart4 for values in (4);
+insert into covidxpart values (4, 1);
+insert into covidxpart values (4, 1);
+ERROR:  duplicate key value violates unique constraint "covidxpart4_a_b_idx"
+DETAIL:  Key (a)=(4) already exists.

diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index ab7c2d147581ee85227eb10b29511eb87237f3bd..29333b31ef2067d962fab35d5b6ed984a059b423 100644 (file)
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -701,3 +701,22 @@ 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);
+
+-- 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);
+create table covidxpart1 partition of covidxpart for values in (1);
+create table covidxpart2 partition of covidxpart for values in (2);
+insert into covidxpart values (1, 1);
+insert into covidxpart values (1, 1);
+create table covidxpart3 (b int, c int, a int);
+alter table covidxpart3 drop c;
+alter table covidxpart attach partition covidxpart3 for values in (3);
+insert into covidxpart values (3, 1);
+insert into covidxpart values (3, 1);
+create table covidxpart4 (b int, a int);
+create unique index on covidxpart4 (a) include (b);
+create unique index on covidxpart4 (a);
+alter table covidxpart attach partition covidxpart4 for values in (4);
+insert into covidxpart values (4, 1);
+insert into covidxpart values (4, 1);