diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 30390dc06..94a13ff01 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -131,6 +131,7 @@ static void ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree); static void ErrorIfUnsupportedIndexStmt(IndexStmt *createIndexStatement); static void ErrorIfUnsupportedDropIndexStmt(DropStmt *dropIndexStatement); static void ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement); +static void ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement); static void ErrorIfUnsupportedSeqStmt(CreateSeqStmt *createSeqStmt); static void ErrorIfDistributedAlterSeqOwnedBy(AlterSeqStmt *alterSeqStmt); static void ErrorIfUnsupportedTruncateStmt(TruncateStmt *truncateStatement); @@ -146,6 +147,8 @@ static void ErrorIfUnsupportedForeignConstraint(Relation relation, static char * ExtractNewExtensionVersion(Node *parsetree); static void CreateLocalTable(RangeVar *relation, char *nodeName, int32 nodePort); static bool IsAlterTableRenameStmt(RenameStmt *renameStmt); +static bool AlterInvolvesPartitionColumn(AlterTableStmt *alterTableStatement, + AlterTableCmd *command); static void ExecuteDistributedDDLJob(DDLJob *ddlJob); static void ShowNoticeIfNotUsing2PC(void); static List * DDLTaskList(Oid relationId, const char *commandString); @@ -401,8 +404,9 @@ multi_ProcessUtility(PlannedStmt *pstmt, /* * citus.enable_ddl_propagation is disabled, which means that PostgreSQL * should handle the DDL command on a distributed table directly, without - * Citus intervening. Advanced Citus users use this to implement their own - * DDL propagation. We also use it to avoid re-propagating DDL commands + * Citus intervening. The only exception is partition column drop, in + * which case we error out. Advanced Citus users use this to implement their + * own DDL propagation. We also use it to avoid re-propagating DDL commands * when changing MX tables on workers. Below, we also make sure that DDL * commands don't run queries that might get intercepted by Citus and error * out, specifically we skip validation in foreign keys. @@ -413,6 +417,8 @@ multi_ProcessUtility(PlannedStmt *pstmt, AlterTableStmt *alterTableStmt = (AlterTableStmt *) parsetree; if (alterTableStmt->relkind == OBJECT_TABLE) { + ErrorIfAlterDropsPartitionColumn(alterTableStmt); + /* * When issuing an ALTER TABLE ... ADD FOREIGN KEY command, the * the validation step should be skipped on the distributed table. @@ -1718,37 +1724,11 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement) case AT_AlterColumnType: case AT_DropNotNull: { - /* error out if the alter table command is on the partition column */ - - Var *partitionColumn = NULL; - HeapTuple tuple = NULL; - char *alterColumnName = command->name; - - LOCKMODE lockmode = AlterTableGetLockLevel(alterTableStatement->cmds); - Oid relationId = AlterTableLookupRelation(alterTableStatement, lockmode); - if (!OidIsValid(relationId)) + if (AlterInvolvesPartitionColumn(alterTableStatement, command)) { - continue; + ereport(ERROR, (errmsg("cannot execute ALTER TABLE command " + "involving partition column"))); } - - partitionColumn = DistPartitionKey(relationId); - - tuple = SearchSysCacheAttName(relationId, alterColumnName); - if (HeapTupleIsValid(tuple)) - { - Form_pg_attribute targetAttr = (Form_pg_attribute) GETSTRUCT(tuple); - - /* reference tables do not have partition column, so allow them */ - if (partitionColumn != NULL && - targetAttr->attnum == partitionColumn->varattno) - { - ereport(ERROR, (errmsg("cannot execute ALTER TABLE command " - "involving partition column"))); - } - - ReleaseSysCache(tuple); - } - break; } @@ -1805,6 +1785,56 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement) } +/* + * ErrorIfDropPartitionColumn checks if any subcommands of the given alter table + * command is a DROP COLUMN command which drops the partition column of a distributed + * table. If there is such a subcommand, this function errors out. + */ +static void +ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement) +{ + LOCKMODE lockmode = 0; + Oid leftRelationId = InvalidOid; + bool isDistributedRelation = false; + List *commandList = alterTableStatement->cmds; + ListCell *commandCell = NULL; + + /* first check whether a distributed relation is affected */ + if (alterTableStatement->relation == NULL) + { + return; + } + + lockmode = AlterTableGetLockLevel(alterTableStatement->cmds); + leftRelationId = AlterTableLookupRelation(alterTableStatement, lockmode); + if (!OidIsValid(leftRelationId)) + { + return; + } + + isDistributedRelation = IsDistributedTable(leftRelationId); + if (!isDistributedRelation) + { + return; + } + + /* then check if any of subcommands drop partition column.*/ + foreach(commandCell, commandList) + { + AlterTableCmd *command = (AlterTableCmd *) lfirst(commandCell); + AlterTableType alterTableType = command->subtype; + if (alterTableType == AT_DropColumn) + { + if (AlterInvolvesPartitionColumn(alterTableStatement, command)) + { + ereport(ERROR, (errmsg("cannot execute ALTER TABLE command " + "dropping partition column"))); + } + } + } +} + + /* * ErrorIfUnsopprtedAlterAddConstraintStmt runs the constraint checks on distributed * table using the same logic with create_distributed_table. @@ -2481,6 +2511,47 @@ IsAlterTableRenameStmt(RenameStmt *renameStmt) } +/* + * AlterInvolvesPartitionColumn checks if the given alter table command + * involves relation's partition column. + */ +static bool +AlterInvolvesPartitionColumn(AlterTableStmt *alterTableStatement, + AlterTableCmd *command) +{ + bool involvesPartitionColumn = false; + Var *partitionColumn = NULL; + HeapTuple tuple = NULL; + char *alterColumnName = command->name; + + LOCKMODE lockmode = AlterTableGetLockLevel(alterTableStatement->cmds); + Oid relationId = AlterTableLookupRelation(alterTableStatement, lockmode); + if (!OidIsValid(relationId)) + { + return false; + } + + partitionColumn = DistPartitionKey(relationId); + + tuple = SearchSysCacheAttName(relationId, alterColumnName); + if (HeapTupleIsValid(tuple)) + { + Form_pg_attribute targetAttr = (Form_pg_attribute) GETSTRUCT(tuple); + + /* reference tables do not have partition column, so allow them */ + if (partitionColumn != NULL && + targetAttr->attnum == partitionColumn->varattno) + { + involvesPartitionColumn = true; + } + + ReleaseSysCache(tuple); + } + + return involvesPartitionColumn; +} + + /* * ExecuteDistributedDDLJob simply executes a provided DDLJob in a distributed trans- * action, including metadata sync if needed. If the multi shard commit protocol is diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index b1519e54a..9aeb855d2 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -992,15 +992,6 @@ RecordDistributedRelationDependencies(Oid distributedRelationId, Node *distribut /* dependency from table entry to extension */ recordDependencyOn(&relationAddr, &citusExtensionAddr, DEPENDENCY_NORMAL); - - /* make sure the distribution key column/expression does not just go away */ -#if (PG_VERSION_NUM >= 100000) - recordDependencyOnSingleRelExpr(&relationAddr, distributionKey, distributedRelationId, - DEPENDENCY_NORMAL, DEPENDENCY_NORMAL, false); -#else - recordDependencyOnSingleRelExpr(&relationAddr, distributionKey, distributedRelationId, - DEPENDENCY_NORMAL, DEPENDENCY_NORMAL); -#endif } diff --git a/src/test/regress/input/multi_alter_table_statements.source b/src/test/regress/input/multi_alter_table_statements.source index 58113c28f..246adcc49 100644 --- a/src/test/regress/input/multi_alter_table_statements.source +++ b/src/test/regress/input/multi_alter_table_statements.source @@ -363,7 +363,7 @@ ALTER TABLE lineitem_alter ALTER COLUMN l_orderkey SET STATISTICS 100; ALTER TABLE lineitem_alter DROP CONSTRAINT IF EXISTS non_existent_contraint; ALTER TABLE lineitem_alter SET WITHOUT OIDS; --- even distribution column can be dropped however postgresql prevents this. +-- distribution column still cannot be dropped. ALTER TABLE lineitem_alter DROP COLUMN l_orderkey; -- Even unique indexes on l_partkey (non-partition column) are allowed. diff --git a/src/test/regress/output/multi_alter_table_statements.source b/src/test/regress/output/multi_alter_table_statements.source index 7c9d29510..1f0fe5af2 100644 --- a/src/test/regress/output/multi_alter_table_statements.source +++ b/src/test/regress/output/multi_alter_table_statements.source @@ -758,11 +758,9 @@ ALTER TABLE lineitem_alter ALTER COLUMN l_orderkey SET STATISTICS 100; ALTER TABLE lineitem_alter DROP CONSTRAINT IF EXISTS non_existent_contraint; NOTICE: constraint "non_existent_contraint" of relation "lineitem_alter" does not exist, skipping ALTER TABLE lineitem_alter SET WITHOUT OIDS; --- even distribution column can be dropped however postgresql prevents this. +-- distribution column still cannot be dropped. ALTER TABLE lineitem_alter DROP COLUMN l_orderkey; -ERROR: cannot drop table lineitem_alter column l_orderkey because other objects depend on it -DETAIL: table lineitem_alter depends on table lineitem_alter column l_orderkey -HINT: Use DROP ... CASCADE to drop the dependent objects too. +ERROR: cannot execute ALTER TABLE command dropping partition column -- Even unique indexes on l_partkey (non-partition column) are allowed. -- Citus would have prevented that. CREATE UNIQUE INDEX unique_lineitem_partkey on lineitem_alter(l_partkey);