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. Wyatt, what do you think? -- Michael
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8d25d14772..5597be6e3d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1091,7 +1091,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, RelationGetDescr(parent)); idxstmt = generateClonedIndexStmt(NULL, 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 ce2484fd4e..f63016871c 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -1168,3 +1168,52 @@ insert into defcheck_def values (0, 0); create table defcheck_0 partition of defcheck for values in (0); ERROR: updated partition constraint for default partition "defcheck_def" would be violated by some row drop table defcheck; +-- 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 + Partitioned 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 144eeb480d..e835b65ac4 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -903,3 +903,26 @@ create table defcheck_1 partition of defcheck for values in (1, null); insert into defcheck_def values (0, 0); create table defcheck_0 partition of defcheck for values in (0); drop table defcheck; + +-- 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;
signature.asc
Description: PGP signature
