Hi.
On 2018/04/11 0:36, Teodor Sigaev wrote:
>> Does the attached fix look correct? Haven't checked the fix with
>> ATTACH
>> PARTITION though.
>>
>>
>> Attached patch seems to fix the problem. However, I would rather get
>> rid of modifying stmt->indexParams. That seems to be more logical
>> for me. Also, it would be good to check some covering indexes on
>> partitioned tables. See the attached patch.
>
> Seems right way, do not modify incoming object and do not copy rather
> large and deep nested structure as suggested by Amit.
Yeah, Alexander's suggested way of using a separate variable for
indexParams is better.
> But it will be better to have a ATTACH PARTITION test too.
I have added tests. Actually, instead of modifying existing tests, I
think it might be better to have a separate section at the end of
indexing.sql to test covering indexes feature for partitioned tables.
Attached find updated patch.
Thanks,
Amit
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 860a60d109..ad819177d7 100644
--- 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 33f68aab71..2c2bf44aa8 100644
--- 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 ab7c2d1475..29333b31ef 100644
--- 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);
