Hi.
I noticed a bug with creating an expression index on a partitioned table.
The fact that a partition may have differing attribute numbers is not
accounted for when recursively *creating* the index on partition. The
partition index gets created but is unusable.
create table p (a int) partition by hash (a);
create table p1 partition of p for values with (modulus 3, remainder 0);
create table p2 partition of p for values with (modulus 3, remainder 1);
create table p3 partition of p for values with (modulus 3, remainder 2);
-- make p3 have different attribute number for column a
alter table p detach partition p3;
alter table p3 drop a, add a int;
alter table p attach partition p3 for values with (modulus 3, remainder 2);
create index on p ((a+1));
\d p3
ERROR: invalid attnum 1 for relation "p3"
That's because the IndexStmt that's passed for creating the index on p3
contains expressions whose Vars are exact copies of the parent's Vars,
which in this case is wrong. We must have converted them to p3's
attribute numbers before calling DefineIndex on p3.
Note that if the same index already exists in a partition before creating
the parent's index, then that index already contains the right Vars, no
conversion is needed in that case.
alter table p detach partition p3;
alter table p3 drop a, add a int;
create index on p3 ((a+1));
alter table p attach partition p3 for values with (modulus 3, remainder 2);
\d p3
Table "public.p3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: p FOR VALUES WITH (modulus 3, remainder 2)
Indexes:
"p3_expr_idx" btree ((a + 1))
create index on p ((a+1));
\d p3
Table "public.p3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: p FOR VALUES WITH (modulus 3, remainder 2)
Indexes:
"p3_expr_idx" btree ((a + 1))
In other words, the code that checks whether an index with matching
definition exists in the partition correctly converts Vars before the
actual matching, so is able to conclude that, despite different Vars in
p3's index, it is same index as the one being created in the parent. So,
the index is not created again, which actually if it were, it would be hit
by the very same bug I'm reporting.
Also is right the case where the partition being attached doesn't have the
index but the parent has and it is correctly *cloned* to the attached
partition.
alter table p detach partition p3;
alter table p3 drop a, add a int;
create index on p ((a+1));
alter table p attach partition p3 for values with (modulus 3, remainder 2);
\d p3
Table "public.p3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: p FOR VALUES WITH (modulus 3, remainder 2)
Indexes:
"p3_expr_idx" btree ((a + 1))
So, CompareIndexInfo and generateClonedIndexStmt are both doing the right
thing, but DefineIndex is not. Attached is a patch to fix DefineIndex so
that it converts indexParams before recursing to create the index on a
partition.
By the way, do we want to do anything about the fact that we don't check
if a matching inherited index exists when creating an index directly on
the partition. I wondered this because we do check the other way around.
\d p3
Table "public.p3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: p FOR VALUES WITH (modulus 3, remainder 2)
Indexes:
"p3_expr_idx" btree ((a + 1))
create index on p3 ((a+1));
\d p3
Table "public.p3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: p FOR VALUES WITH (modulus 3, remainder 2)
Indexes:
"p3_expr_idx" btree ((a + 1))
"p3_expr_idx1" btree ((a + 1))
Thanks,
Amit
From d7c090f1c14a5d5ebec04b5a58d626cbfe057e2c Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 21 Jun 2018 14:50:20 +0900
Subject: [PATCH v1] Convert indexParams to partition's attnos before recursing
---
src/backend/commands/indexcmds.c | 18 +++++++++++++-
src/test/regress/expected/indexing.out | 45 ++++++++++++++++++++++++++++++++++
src/test/regress/sql/indexing.sql | 26 ++++++++++++++++++++
3 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 3a3223bffb..74ffd2c392 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -922,7 +922,6 @@ DefineIndex(Oid relationId,
gettext_noop("could not convert row type"));
maplen = parentDesc->natts;
-
foreach(cell, childidxs)
{
Oid cldidxid =
lfirst_oid(cell);
@@ -993,7 +992,24 @@ DefineIndex(Oid relationId,
{
IndexStmt *childStmt =
copyObject(stmt);
bool found_whole_row;
+ ListCell *lc;
+ /* Adjust Vars to match new table's
column numbering */
+ foreach(lc, childStmt->indexParams)
+ {
+ IndexElem *ielem = lfirst(lc);
+
+ /*
+ * If the index parameter is an
expression, we must
+ * translate it to contain
child Vars.
+ */
+ if (ielem->expr)
+ ielem->expr =
+
map_variable_attnos((Node *) ielem->expr,
+
1, 0, attmap, maplen,
+
InvalidOid,
+
&found_whole_row);
+ }
childStmt->whereClause =
map_variable_attnos(stmt->whereClause, 1, 0,
attmap, maplen,
diff --git a/src/test/regress/expected/indexing.out
b/src/test/regress/expected/indexing.out
index 2c2bf44aa8..5a9c3ee943 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1337,3 +1337,48 @@ 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.
+-- tests covering expression indexes in a partition tree with differing
attributes
+create table idx_part (a int) partition by hash (a);
+create table idx_part1 partition of idx_part for values with (modulus 2,
remainder 0);
+create table idx_part2 partition of idx_part for values with (modulus 2,
remainder 1);
+alter table idx_part detach partition idx_part2;
+alter table idx_part2 drop a, add a int;
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+create index idx_part_expr on idx_part ((a + 1));
+\d idx_part2
+ Table "public.idx_part2"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: idx_part FOR VALUES WITH (modulus 2, remainder 1)
+Indexes:
+ "idx_part2_expr_idx" btree ((a + 1))
+
+drop index idx_part_expr;
+alter table idx_part detach partition idx_part2;
+create index idx_part2_expr on idx_part2 ((a + 1));
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+create index idx_part_expr on idx_part ((a + 1));
+\d idx_part2
+ Table "public.idx_part2"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: idx_part FOR VALUES WITH (modulus 2, remainder 1)
+Indexes:
+ "idx_part2_expr" btree ((a + 1))
+
+drop table idx_part2;
+create table idx_part2 (b int, a int);
+alter table idx_part2 drop b;
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+\d idx_part2;
+ Table "public.idx_part2"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: idx_part FOR VALUES WITH (modulus 2, remainder 1)
+Indexes:
+ "idx_part2_expr_idx" btree ((a + 1))
+
+drop table idx_part;
diff --git a/src/test/regress/sql/indexing.sql
b/src/test/regress/sql/indexing.sql
index 29333b31ef..b050781e00 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -720,3 +720,29 @@ 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);
+
+-- tests covering expression indexes in a partition tree with differing
attributes
+create table idx_part (a int) partition by hash (a);
+create table idx_part1 partition of idx_part for values with (modulus 2,
remainder 0);
+create table idx_part2 partition of idx_part for values with (modulus 2,
remainder 1);
+
+alter table idx_part detach partition idx_part2;
+alter table idx_part2 drop a, add a int;
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+create index idx_part_expr on idx_part ((a + 1));
+\d idx_part2
+
+drop index idx_part_expr;
+alter table idx_part detach partition idx_part2;
+create index idx_part2_expr on idx_part2 ((a + 1));
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+create index idx_part_expr on idx_part ((a + 1));
+\d idx_part2
+
+drop table idx_part2;
+create table idx_part2 (b int, a int);
+alter table idx_part2 drop b;
+alter table idx_part attach partition idx_part2 for values with (modulus 2,
remainder 1);
+\d idx_part2;
+
+drop table idx_part;
--
2.11.0
