From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Teodor Sigaev <teodor@sigaev.ru>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Jaime Casanova <jaime.casanova@2ndquadrant.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Partitioned tables and covering indexes
Date: Wed, 11 Apr 2018 09:50:49 +0900
Message-ID: <994fa677-2dac-8fe5-3f39-024cd6406522@lab.ntt.co.jp> (raw)
In-Reply-To: <a5ca67cf-0448-7875-ae89-64495748c016@sigaev.ru>
References: <CAJGNTeO=BguEyG8wxMpU_Vgvg3nGGzy71zUQ0RpzEn_mb0bSWA@mail.gmail.com>
<48893c03-d1c8-62ef-b37d-b2f7dbdb8245@lab.ntt.co.jp>
<CAPpHfdujMkQmhhzKG0sPB=27V9GXJeSVBGHt1rhHCPK89OfKfw@mail.gmail.com>
<a5ca67cf-0448-7875-ae89-64495748c016@sigaev.ru>
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
Attachments:DefineIndex-fix-covering-index-partitioned-3.patch (text/plain, 5.9K) downloaddiff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 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.outindex 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.sqlindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: Langote_Amit_f8@lab.ntt.co.jp, teodor@sigaev.ru, a.korotkov@postgrespro.ru, jaime.casanova@2ndquadrant.com
Subject: Re: Partitioned tables and covering indexes
In-Reply-To: <994fa677-2dac-8fe5-3f39-024cd6406522@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox