On Tue, Oct 08, 2019 at 06:25:05PM +0900, Amit Langote wrote: > I thought about doing something like that, but wasn't sure if > introducing that much complexity is warranted.
I looked at that. By experience, I think that it would be wiser to do first the lookup of all the dependencies you would like to delete, and then let the internal dependency machinery sort things out after recursing (remember recent fixes related to ON COMMIT actions). In order to do that, you actually just need to be careful to not trigger the deletions as long as "recursing" is true because ATExecDropColumn calls itself. And it is not actually as bad as I assumed, please see the attached. -- Michael
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05593f3316..87c39ad6df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -401,7 +401,8 @@ static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool rec
static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
bool recurse, bool recursing,
- bool missing_ok, LOCKMODE lockmode);
+ bool missing_ok, LOCKMODE lockmode,
+ ObjectAddresses *addrs);
static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode);
static ObjectAddress ATExecAddConstraint(List **wqueue,
@@ -4273,12 +4274,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
case AT_DropColumn: /* DROP COLUMN */
address = ATExecDropColumn(wqueue, rel, cmd->name,
cmd->behavior, false, false,
- cmd->missing_ok, lockmode);
+ cmd->missing_ok, lockmode,
+ NULL);
break;
case AT_DropColumnRecurse: /* DROP COLUMN with recursion */
address = ATExecDropColumn(wqueue, rel, cmd->name,
cmd->behavior, true, false,
- cmd->missing_ok, lockmode);
+ cmd->missing_ok, lockmode,
+ NULL);
break;
case AT_AddIndex: /* ADD INDEX */
address = ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false,
@@ -7004,12 +7007,16 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
/*
* Return value is the address of the dropped column.
+ *
+ * *addrs stores the object addresses of all the columns to delete in
+ * case of a recursing lookup.
*/
static ObjectAddress
ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
bool recurse, bool recursing,
- bool missing_ok, LOCKMODE lockmode)
+ bool missing_ok, LOCKMODE lockmode,
+ ObjectAddresses *addrs)
{
HeapTuple tuple;
Form_pg_attribute targetatt;
@@ -7022,6 +7029,13 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
if (recursing)
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ /*
+ * Initialize the location of addresses which will be deleted on a
+ * recursing inheritance lookup.
+ */
+ if (!recursing)
+ addrs = new_object_addresses();
+
/*
* get the number of the attribute
*/
@@ -7134,7 +7148,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
/* Time to delete this child column, too */
ATExecDropColumn(wqueue, childrel, colName,
behavior, true, true,
- false, lockmode);
+ false, lockmode, addrs);
}
else
{
@@ -7170,14 +7184,20 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
table_close(attr_rel, RowExclusiveLock);
}
- /*
- * Perform the actual column deletion
- */
+ /* Add object to delete */
object.classId = RelationRelationId;
object.objectId = RelationGetRelid(rel);
object.objectSubId = attnum;
+ add_exact_object_address(&object, addrs);
- performDeletion(&object, behavior, 0);
+ /*
+ * The recursion is ending, hence perform the actual column deletions.
+ */
+ if (!recursing)
+ {
+ performMultipleDeletions(addrs, behavior, 0);
+ free_object_addresses(addrs);
+ }
return object;
}
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index c143df5114..28498b220d 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1258,3 +1258,20 @@ ERROR: cannot drop inherited constraint "parted_uniq_detach_test1_a_key" of rel
alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
drop table parted_uniq_detach_test, parted_uniq_detach_test1;
+-- check that dropping a column takes with it any partitioned indexes
+-- depending on it
+create table parted_index_col_drop(a int, b int, c int) partition by list (a);
+create table parted_index_col_drop1 partition of parted_index_col_drop for values in (1) partition by list (a);
+create table parted_index_col_drop11 partition of parted_index_col_drop1 for values in (1);
+create index on parted_index_col_drop (b, c);
+alter table parted_index_col_drop drop c;
+\d parted_index_col_drop
+ Partitioned table "public.parted_index_col_drop"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+Partition key: LIST (a)
+Number of partitions: 1 (Use \d+ to list them.)
+
+drop table parted_index_col_drop;
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index cc3d0abfb7..8ba054b907 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -703,3 +703,13 @@ alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_
alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
drop table parted_uniq_detach_test, parted_uniq_detach_test1;
+
+-- check that dropping a column takes with it any partitioned indexes
+-- depending on it
+create table parted_index_col_drop(a int, b int, c int) partition by list (a);
+create table parted_index_col_drop1 partition of parted_index_col_drop for values in (1) partition by list (a);
+create table parted_index_col_drop11 partition of parted_index_col_drop1 for values in (1);
+create index on parted_index_col_drop (b, c);
+alter table parted_index_col_drop drop c;
+\d parted_index_col_drop
+drop table parted_index_col_drop;
signature.asc
Description: PGP signature
