From 17266e3301dd7c39fa8a41ae79fc6c4f9fea7398 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Thu, 11 Jan 2018 18:05:51 +0100 Subject: [PATCH] Implement ALTER INDEX ... RENAME TO ... The command is now distributed among the shards when the table is distributed. To that effect, we fill in the DDLJob's targetRelationId with the OID of the table for which the index is defined, rather than the OID of the index itself. --- .../distributed/executor/multi_utility.c | 89 ++++++++++++++++--- .../input/multi_alter_table_statements.source | 15 ++++ .../multi_alter_table_statements.source | 34 +++++++ 3 files changed, 126 insertions(+), 12 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 519954eea..51f2d89a6 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -155,6 +155,7 @@ 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 IsIndexRenameStmt(RenameStmt *renameStmt); static bool AlterInvolvesPartitionColumn(AlterTableStmt *alterTableStatement, AlterTableCmd *command); static void ExecuteDistributedDDLJob(DDLJob *ddlJob); @@ -1371,11 +1372,17 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo static List * PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand) { - Oid relationId = InvalidOid; + Oid objectRelationId = InvalidOid; /* SQL Object OID */ + Oid tableRelationId = InvalidOid; /* Relation OID, maybe not the same. */ bool isDistributedRelation = false; DDLJob *ddlJob = NULL; - if (!IsAlterTableRenameStmt(renameStmt)) + /* + * We only support some of the PostgreSQL supported RENAME statements, and + * our list include only renaming table and index (related) objects. + */ + if (!IsAlterTableRenameStmt(renameStmt) && + !IsIndexRenameStmt(renameStmt)) { return NIL; } @@ -1385,32 +1392,70 @@ PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand) * RenameRelation(), renameatt() and RenameConstraint(). However, since all * three statements have identical lock levels, we just use a single statement. */ - relationId = RangeVarGetRelid(renameStmt->relation, AccessExclusiveLock, - renameStmt->missing_ok); + objectRelationId = RangeVarGetRelid(renameStmt->relation, + AccessExclusiveLock, + renameStmt->missing_ok); /* * If the table does not exist, don't do anything here to allow PostgreSQL * to throw the appropriate error or notice message later. */ - if (!OidIsValid(relationId)) + if (!OidIsValid(objectRelationId)) { return NIL; } /* we have no planning to do unless the table is distributed */ - isDistributedRelation = IsDistributedTable(relationId); + switch (renameStmt->renameType) + { + case OBJECT_TABLE: + case OBJECT_COLUMN: + case OBJECT_TABCONSTRAINT: + { + /* the target object is our tableRelationId. */ + tableRelationId = objectRelationId; + break; + } + + case OBJECT_INDEX: + { + /* + * here, objRelationId points to the index relation entry, and we + * are interested into the entry of the table on which the index is + * defined. + */ + tableRelationId = IndexGetRelation(objectRelationId, false); + break; + } + + default: + + /* + * Nodes that are not supported by Citus: we pass-through to the + * main PostgreSQL executor. Any Citus-supported RenameStmt + * renameType must appear above in the switch, explicitly. + */ + return NIL; + } + + isDistributedRelation = IsDistributedTable(tableRelationId); if (!isDistributedRelation) { return NIL; } + /* + * We might ERROR out on some commands, but only for Citus tables where + * isDistributedRelation is true. That's why this test comes this late in + * the function. + */ ErrorIfUnsupportedRenameStmt(renameStmt); ddlJob = palloc0(sizeof(DDLJob)); - ddlJob->targetRelationId = relationId; + ddlJob->targetRelationId = tableRelationId; ddlJob->concurrentIndexCmd = false; ddlJob->commandString = renameCommand; - ddlJob->taskList = DDLTaskList(relationId, renameCommand); + ddlJob->taskList = DDLTaskList(tableRelationId, renameCommand); return list_make1(ddlJob); } @@ -2733,14 +2778,14 @@ OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId) * ErrorIfDistributedRenameStmt errors out if the corresponding rename statement * operates on any part of a distributed table other than a column. * - * Note: This function handles only those rename statements which operate on tables. + * Note: This function handles RenameStmt applied to relations handed by Citus. + * At the moment of writing this comment, it could be either tables or indexes. */ static void ErrorIfUnsupportedRenameStmt(RenameStmt *renameStmt) { - Assert(IsAlterTableRenameStmt(renameStmt)); - - if (renameStmt->renameType == OBJECT_TABCONSTRAINT) + if (IsAlterTableRenameStmt(renameStmt) && + renameStmt->renameType == OBJECT_TABCONSTRAINT) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("renaming constraints belonging to distributed tables is " @@ -2860,6 +2905,26 @@ IsAlterTableRenameStmt(RenameStmt *renameStmt) } +/* + * IsIndexRenameStmt returns whether the passed-in RenameStmt is the following + * form: + * + * - ALTER INDEX RENAME + */ +static bool +IsIndexRenameStmt(RenameStmt *renameStmt) +{ + bool isIndexRenameStmt = false; + + if (renameStmt->renameType == OBJECT_INDEX) + { + isIndexRenameStmt = true; + } + + return isIndexRenameStmt; +} + + /* * AlterInvolvesPartitionColumn checks if the given alter table command * involves relation's partition column. diff --git a/src/test/regress/input/multi_alter_table_statements.source b/src/test/regress/input/multi_alter_table_statements.source index 2cefeaf88..ababf5119 100644 --- a/src/test/regress/input/multi_alter_table_statements.source +++ b/src/test/regress/input/multi_alter_table_statements.source @@ -375,6 +375,21 @@ ALTER TABLE lineitem_renamed RENAME TO lineitem_alter; SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname; \c - - - :master_port +-- verify that we can rename indexes on distributed tables +CREATE INDEX temp_index_1 ON lineitem_alter(l_linenumber); +ALTER INDEX temp_index_1 RENAME TO idx_lineitem_linenumber; + +-- verify rename is performed +SELECT relname FROM pg_class WHERE relname = 'idx_lineitem_linenumber'; + +-- show rename worked on one worker, too +\c - - - :worker_1_port +SELECT relname FROM pg_class WHERE relname LIKE 'idx_lineitem_linenumber%%' ORDER BY relname; +\c - - - :master_port + +-- now get rid of the index +DROP INDEX idx_lineitem_linenumber; + -- verify that we don't intercept DDL commands if propagation is turned off SET citus.enable_ddl_propagation to false; diff --git a/src/test/regress/output/multi_alter_table_statements.source b/src/test/regress/output/multi_alter_table_statements.source index 49017ff2e..b00f53ca0 100644 --- a/src/test/regress/output/multi_alter_table_statements.source +++ b/src/test/regress/output/multi_alter_table_statements.source @@ -844,6 +844,40 @@ SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relna (15 rows) \c - - - :master_port +-- verify that we can rename indexes on distributed tables +CREATE INDEX temp_index_1 ON lineitem_alter(l_linenumber); +ALTER INDEX temp_index_1 RENAME TO idx_lineitem_linenumber; +-- verify rename is performed +SELECT relname FROM pg_class WHERE relname = 'idx_lineitem_linenumber'; + relname +------------------------- + idx_lineitem_linenumber +(1 row) + +-- show rename worked on one worker, too +\c - - - :worker_1_port +SELECT relname FROM pg_class WHERE relname LIKE 'idx_lineitem_linenumber%%' ORDER BY relname; + relname +-------------------------------- + idx_lineitem_linenumber_220000 + idx_lineitem_linenumber_220001 + idx_lineitem_linenumber_220002 + idx_lineitem_linenumber_220003 + idx_lineitem_linenumber_220004 + idx_lineitem_linenumber_220005 + idx_lineitem_linenumber_220006 + idx_lineitem_linenumber_220007 + idx_lineitem_linenumber_220008 + idx_lineitem_linenumber_220010 + idx_lineitem_linenumber_220011 + idx_lineitem_linenumber_220012 + idx_lineitem_linenumber_220013 + idx_lineitem_linenumber_220014 +(14 rows) + +\c - - - :master_port +-- now get rid of the index +DROP INDEX idx_lineitem_linenumber; -- verify that we don't intercept DDL commands if propagation is turned off SET citus.enable_ddl_propagation to false; -- table rename statement can be performed on the coordinator only now