On Thu, Oct 31, 2019 at 9:45 AM Michael Paquier <mich...@paquier.xyz> wrote:
> On Tue, Oct 29, 2019 at 01:16:58PM +0900, Michael Paquier wrote:
> > Yes, something looks wrong with that. I have not looked at it in
> > details yet though. I'll see about that tomorrow.
>
> So.. When building the attribute map for a cloned index (with either
> LIKE during the transformation or for partition indexes), we store
> each attribute number with 0 used for dropped columns. Unfortunately,
> if you look at the way the attribute map is built in this case the
> code correctly generates the mapping with convert_tuples_by_name_map.
> But, the length of the mapping used is incorrect as this makes use of
> the number of attributes for the newly-created child relation, and not
> the parent which includes the dropped column in its count. So the
> answer is simply to use the parent as reference for the mapping
> length.
>
> The patch is rather simple as per the attached, with extended
> regression tests included. I have not checked on back-branches yet,
> but that's visibly wrong since 8b08f7d down to v11 (will do that when
> back-patching).
>
> There could be a point in changing convert_tuples_by_name_map & co so
> as they return the length of the map on top of the map to avoid such
> mistakes in the future. That's a more invasive patch not really
> adapted for a back-patch, but we could do that on HEAD once this bug
> is fixed. I have also checked other calls of this API and the
> handling is done correctly.
>
> The patch works for me on master and on 12. I have rebased the patch for
Version 11.
> Wyatt, what do you think?
> --
> Michael
>
--
Ibrar Ahmed
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b53f6ed3ac..9a1ef78a1f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -961,7 +961,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
gettext_noop("could not convert row type"));
idxstmt =
generateClonedIndexStmt(NULL, RelationGetRelid(rel), idxRel,
- attmap, RelationGetDescr(rel)->natts,
+ attmap, RelationGetDescr(parent)->natts,
&constraintOid);
DefineIndex(RelationGetRelid(rel),
idxstmt,
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index c25117e47c..d20ba3dbdf 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -1006,3 +1006,52 @@ CONTEXT: SQL statement "create table tab_part_create_1 partition of tab_part_cr
PL/pgSQL function func_part_create() line 3 at EXECUTE
drop table tab_part_create;
drop function func_part_create();
+-- tests of column drop with partition tables and indexes using
+-- predicates and expressions.
+create table part_column_drop (
+ useless_1 int,
+ id int,
+ useless_2 int,
+ d int,
+ b int,
+ useless_3 int
+) partition by range (id);
+alter table part_column_drop drop column useless_1;
+alter table part_column_drop drop column useless_2;
+alter table part_column_drop drop column useless_3;
+create index part_column_drop_b_pred on part_column_drop(b) where b = 1;
+create index part_column_drop_b_expr on part_column_drop((b = 1));
+create index part_column_drop_d_pred on part_column_drop(d) where d = 2;
+create index part_column_drop_d_expr on part_column_drop((d = 2));
+create table part_column_drop_1_10 partition of
+ part_column_drop for values from (1) to (10);
+\d part_column_drop
+ Table "public.part_column_drop"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ id | integer | | |
+ d | integer | | |
+ b | integer | | |
+Partition key: RANGE (id)
+Indexes:
+ "part_column_drop_b_expr" btree ((b = 1))
+ "part_column_drop_b_pred" btree (b) WHERE b = 1
+ "part_column_drop_d_expr" btree ((d = 2))
+ "part_column_drop_d_pred" btree (d) WHERE d = 2
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d part_column_drop_1_10
+ Table "public.part_column_drop_1_10"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ id | integer | | |
+ d | integer | | |
+ b | integer | | |
+Partition of: part_column_drop FOR VALUES FROM (1) TO (10)
+Indexes:
+ "part_column_drop_1_10_b_idx" btree (b) WHERE b = 1
+ "part_column_drop_1_10_d_idx" btree (d) WHERE d = 2
+ "part_column_drop_1_10_expr_idx" btree ((b = 1))
+ "part_column_drop_1_10_expr_idx1" btree ((d = 2))
+
+drop table part_column_drop;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 27e6f59e87..601a4b9dc3 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -814,3 +814,26 @@ create trigger trig_part_create before insert on tab_part_create
insert into tab_part_create values (1);
drop table tab_part_create;
drop function func_part_create();
+
+-- tests of column drop with partition tables and indexes using
+-- predicates and expressions.
+create table part_column_drop (
+ useless_1 int,
+ id int,
+ useless_2 int,
+ d int,
+ b int,
+ useless_3 int
+) partition by range (id);
+alter table part_column_drop drop column useless_1;
+alter table part_column_drop drop column useless_2;
+alter table part_column_drop drop column useless_3;
+create index part_column_drop_b_pred on part_column_drop(b) where b = 1;
+create index part_column_drop_b_expr on part_column_drop((b = 1));
+create index part_column_drop_d_pred on part_column_drop(d) where d = 2;
+create index part_column_drop_d_expr on part_column_drop((d = 2));
+create table part_column_drop_1_10 partition of
+ part_column_drop for values from (1) to (10);
+\d part_column_drop
+\d part_column_drop_1_10
+drop table part_column_drop;
