Skip to content

Commit 3a58c5f

Browse files
michaelpqmichaelpq
committed
Fix dependency handling of column drop with partitioned tables
When dropping a column on a partitioned table which has one or more partitioned indexes, the operation was failing as dependencies with partitioned indexes using the column dropped were not getting removed in a way consistent with the columns involved across all the relations part of an inheritance tree. This commit refactors the code executing column drop so as all the columns from an inheritance tree to remove are gathered first, and dropped all at the end. This way, we let the dependency machinery sort out by itself the deletion of all the columns with the partitioned indexes across a partition tree. This issue has been introduced by 1d92a0c, so backpatch down to REL_12_STABLE. Author: Amit Langote, Michael Paquier Reviewed-by: Álvaro Herrera, Ashutosh Sharma Discussion: https://postgr.es/m/CA+HiwqE9kuBsZ3b5pob2-cvE8ofzPWs-og+g8bKKGnu6b4-yTQ@mail.gmail.com Backpatch-through: 12
1 parent 3fb14cf commit 3a58c5f

File tree

3 files changed

+113
-10
lines changed

3 files changed

+113
-10
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,8 @@ static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool rec
401401
static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
402402
DropBehavior behavior,
403403
bool recurse, bool recursing,
404-
bool missing_ok, LOCKMODE lockmode);
404+
bool missing_ok, LOCKMODE lockmode,
405+
ObjectAddresses *addrs);
405406
static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
406407
IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode);
407408
static ObjectAddress ATExecAddConstraint(List **wqueue,
@@ -4276,12 +4277,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
42764277
case AT_DropColumn: /* DROP COLUMN */
42774278
address = ATExecDropColumn(wqueue, rel, cmd->name,
42784279
cmd->behavior, false, false,
4279-
cmd->missing_ok, lockmode);
4280+
cmd->missing_ok, lockmode,
4281+
NULL);
42804282
break;
42814283
case AT_DropColumnRecurse: /* DROP COLUMN with recursion */
42824284
address = ATExecDropColumn(wqueue, rel, cmd->name,
42834285
cmd->behavior, true, false,
4284-
cmd->missing_ok, lockmode);
4286+
cmd->missing_ok, lockmode,
4287+
NULL);
42854288
break;
42864289
case AT_AddIndex: /* ADD INDEX */
42874290
address = ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false,
@@ -7016,13 +7019,22 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
70167019
}
70177020

70187021
/*
7019-
* Return value is the address of the dropped column.
7022+
* Drops column 'colName' from relation 'rel' and returns the address of the
7023+
* dropped column. The column is also dropped (or marked as no longer
7024+
* inherited from relation) from the relation's inheritance children, if any.
7025+
*
7026+
* In the recursive invocations for inheritance child relations, instead of
7027+
* dropping the column directly (if to be dropped at all), its object address
7028+
* is added to 'addrs', which must be non-NULL in such invocations. All
7029+
* columns are dropped at the same time after all the children have been
7030+
* checked recursively.
70207031
*/
70217032
static ObjectAddress
70227033
ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
70237034
DropBehavior behavior,
70247035
bool recurse, bool recursing,
7025-
bool missing_ok, LOCKMODE lockmode)
7036+
bool missing_ok, LOCKMODE lockmode,
7037+
ObjectAddresses *addrs)
70267038
{
70277039
HeapTuple tuple;
70287040
Form_pg_attribute targetatt;
@@ -7035,6 +7047,11 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
70357047
if (recursing)
70367048
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
70377049

7050+
/* Initialize addrs on the first invocation */
7051+
Assert(!recursing || addrs != NULL);
7052+
if (!recursing)
7053+
addrs = new_object_addresses();
7054+
70387055
/*
70397056
* get the number of the attribute
70407057
*/
@@ -7147,7 +7164,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
71477164
/* Time to delete this child column, too */
71487165
ATExecDropColumn(wqueue, childrel, colName,
71497166
behavior, true, true,
7150-
false, lockmode);
7167+
false, lockmode, addrs);
71517168
}
71527169
else
71537170
{
@@ -7183,14 +7200,18 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
71837200
table_close(attr_rel, RowExclusiveLock);
71847201
}
71857202

7186-
/*
7187-
* Perform the actual column deletion
7188-
*/
7203+
/* Add object to delete */
71897204
object.classId = RelationRelationId;
71907205
object.objectId = RelationGetRelid(rel);
71917206
object.objectSubId = attnum;
7207+
add_exact_object_address(&object, addrs);
71927208

7193-
performDeletion(&object, behavior, 0);
7209+
if (!recursing)
7210+
{
7211+
/* Recursion has ended, drop everything that was collected */
7212+
performMultipleDeletions(addrs, behavior, 0);
7213+
free_object_addresses(addrs);
7214+
}
71947215

71957216
return object;
71967217
}

‎src/test/regress/expected/indexing.out

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,3 +1258,64 @@ ERROR: cannot drop inherited constraint "parted_uniq_detach_test1_a_key" of rel
12581258
alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
12591259
alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
12601260
drop table parted_uniq_detach_test, parted_uniq_detach_test1;
1261+
-- check that dropping a column takes with it any partitioned indexes
1262+
-- depending on it.
1263+
create table parted_index_col_drop(a int, b int, c int)
1264+
partition by list (a);
1265+
create table parted_index_col_drop1 partition of parted_index_col_drop
1266+
for values in (1) partition by list (a);
1267+
-- leave this partition without children.
1268+
create table parted_index_col_drop2 partition of parted_index_col_drop
1269+
for values in (2) partition by list (a);
1270+
create table parted_index_col_drop11 partition of parted_index_col_drop1
1271+
for values in (1);
1272+
create index on parted_index_col_drop (b);
1273+
create index on parted_index_col_drop (c);
1274+
create index on parted_index_col_drop (b, c);
1275+
alter table parted_index_col_drop drop column c;
1276+
\d parted_index_col_drop
1277+
Partitioned table "public.parted_index_col_drop"
1278+
Column | Type | Collation | Nullable | Default
1279+
--------+---------+-----------+----------+---------
1280+
a | integer | | |
1281+
b | integer | | |
1282+
Partition key: LIST (a)
1283+
Indexes:
1284+
"parted_index_col_drop_b_idx" btree (b)
1285+
Number of partitions: 2 (Use \d+ to list them.)
1286+
1287+
\d parted_index_col_drop1
1288+
Partitioned table "public.parted_index_col_drop1"
1289+
Column | Type | Collation | Nullable | Default
1290+
--------+---------+-----------+----------+---------
1291+
a | integer | | |
1292+
b | integer | | |
1293+
Partition of: parted_index_col_drop FOR VALUES IN (1)
1294+
Partition key: LIST (a)
1295+
Indexes:
1296+
"parted_index_col_drop1_b_idx" btree (b)
1297+
Number of partitions: 1 (Use \d+ to list them.)
1298+
1299+
\d parted_index_col_drop2
1300+
Partitioned table "public.parted_index_col_drop2"
1301+
Column | Type | Collation | Nullable | Default
1302+
--------+---------+-----------+----------+---------
1303+
a | integer | | |
1304+
b | integer | | |
1305+
Partition of: parted_index_col_drop FOR VALUES IN (2)
1306+
Partition key: LIST (a)
1307+
Indexes:
1308+
"parted_index_col_drop2_b_idx" btree (b)
1309+
Number of partitions: 0
1310+
1311+
\d parted_index_col_drop11
1312+
Table "public.parted_index_col_drop11"
1313+
Column | Type | Collation | Nullable | Default
1314+
--------+---------+-----------+----------+---------
1315+
a | integer | | |
1316+
b | integer | | |
1317+
Partition of: parted_index_col_drop1 FOR VALUES IN (1)
1318+
Indexes:
1319+
"parted_index_col_drop11_b_idx" btree (b)
1320+
1321+
drop table parted_index_col_drop;

‎src/test/regress/sql/indexing.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,3 +703,24 @@ alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_
703703
alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
704704
alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
705705
drop table parted_uniq_detach_test, parted_uniq_detach_test1;
706+
707+
-- check that dropping a column takes with it any partitioned indexes
708+
-- depending on it.
709+
create table parted_index_col_drop(a int, b int, c int)
710+
partition by list (a);
711+
create table parted_index_col_drop1 partition of parted_index_col_drop
712+
for values in (1) partition by list (a);
713+
-- leave this partition without children.
714+
create table parted_index_col_drop2 partition of parted_index_col_drop
715+
for values in (2) partition by list (a);
716+
create table parted_index_col_drop11 partition of parted_index_col_drop1
717+
for values in (1);
718+
create index on parted_index_col_drop (b);
719+
create index on parted_index_col_drop (c);
720+
create index on parted_index_col_drop (b, c);
721+
alter table parted_index_col_drop drop column c;
722+
\d parted_index_col_drop
723+
\d parted_index_col_drop1
724+
\d parted_index_col_drop2
725+
\d parted_index_col_drop11
726+
drop table parted_index_col_drop;

0 commit comments

Comments
 (0)