external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove distributed tables' dependency on distribution key columns. (#1527) 

This change removes distributed tables' dependency on distribution key columns. We already check that we cannot drop distribution key columns in ErrorIfUnsupportedAlterTableStmt() at multi_utility.c, so we don't need to have distributed table to distribution key column dependency to avoid dropping of distribution key column.

Furthermore, having this dependency causes some warnings in pg_dump --schema-only (See #866), which are not desirable.

This change also adds check to disallow drop of distribution keys when citus.enable_ddl_propagation is set to false. Regression tests are updated accordingly.
pull/1534/head
Hadi Moshayedi 2017-08-03 10:07:04 -04:00 committed by GitHub
parent fa18899cf9
commit 8229a64fe8
4 changed files with 105 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
src/backend/distributed/executor/multi_utility.c
View File

@ -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

9
src/backend/distributed/master/master_metadata_utility.c
View File

@ -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
}

2
src/test/regress/input/multi_alter_table_statements.source
View File

@ -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.

6
src/test/regress/output/multi_alter_table_statements.source
View File

@ -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);