Enable distributed ALTER TABLE ... RENAME COLUMN

Pretty straightforward. Had some concerns about locking, but due to the
fact that all distributed operations use either some level of deparsing
or need to enumerate column names, they all block during any concurrent
column renames (due to the AccessExclusive lock).

In addition, I had some misgivings about permitting renames of the dis-
tribution column, but nothing bad comes from just allowing them.

Finally, I tried to trigger any sort of error using prepared statements
and could not trigger any errors not also exhibited by plain PostgreSQL
tables.

src/backend/distributed/executor/multi_utility.c

@@ -112,6 +112,7 @@ static List * PlanDropIndexStmt(DropStmt *dropIndexStatement,
 								const char *dropIndexCommand);
 static List * PlanAlterTableStmt(AlterTableStmt *alterTableStatement,
 								 const char *alterTableCommand);
+static List * PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand);
 static Node * WorkerProcessAlterTableStmt(AlterTableStmt *alterTableStatement,
 										  const char *alterTableCommand);
 static List * PlanAlterObjectSchemaStmt(AlterObjectSchemaStmt *alterObjectSchemaStmt,
@@ -132,12 +133,12 @@ static void ErrorIfUnsupportedSeqStmt(CreateSeqStmt *createSeqStmt);
 static void ErrorIfDistributedAlterSeqOwnedBy(AlterSeqStmt *alterSeqStmt);
 static void ErrorIfUnsupportedTruncateStmt(TruncateStmt *truncateStatement);
 static bool OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId);
-static void ErrorIfDistributedRenameStmt(RenameStmt *renameStatement);
+static void ErrorIfUnsupportedRenameStmt(RenameStmt *renameStmt);
 
 /* Local functions forward declarations for helper functions */
 static char * ExtractNewExtensionVersion(Node *parsetree);
 static void CreateLocalTable(RangeVar *relation, char *nodeName, int32 nodePort);
-static bool IsAlterTableRenameStmt(RenameStmt *renameStatement);
+static bool IsAlterTableRenameStmt(RenameStmt *renameStmt);
 static void ExecuteDistributedDDLJob(DDLJob *ddlJob);
 static void ShowNoticeIfNotUsing2PC(void);
 static List * DDLTaskList(Oid relationId, const char *commandString);
@@ -290,11 +291,7 @@ multi_ProcessUtility(Node *parsetree,
 		 */
 		if (IsA(parsetree, RenameStmt))
 		{
-			RenameStmt *renameStmt = (RenameStmt *) parsetree;
+			ddlJobs = PlanRenameStmt((RenameStmt *) parsetree, queryString);
-			if (IsAlterTableRenameStmt(renameStmt))
-			{
-				ErrorIfDistributedRenameStmt(renameStmt);
-			}
 		}
 
 		/*
@@ -981,6 +978,61 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo
 }
 
 
+/*
+ * PlanRenameStmt first determines whether a given rename statement involves
+ * a distributed table. If so (and if it is supported, i.e. renames a column),
+ * it creates a DDLJob to encapsulate information needed during the worker node
+ * portion of DDL execution before returning that DDLJob in a List. If no dis-
+ * tributed table is involved, this function returns NIL.
+ */
+static List *
+PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand)
+{
+	Oid relationId = InvalidOid;
+	bool isDistributedRelation = false;
+	DDLJob *ddlJob = NULL;
+
+	if (!IsAlterTableRenameStmt(renameStmt))
+	{
+		return NIL;
+	}
+
+	/*
+	 * The lock levels here should be same as the ones taken in
+	 * 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);
+
+	/*
+	 * 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))
+	{
+		return NIL;
+	}
+
+	/* we have no planning to do unless the table is distributed */
+	isDistributedRelation = IsDistributedTable(relationId);
+	if (!isDistributedRelation)
+	{
+		return NIL;
+	}
+
+	ErrorIfUnsupportedRenameStmt(renameStmt);
+
+	ddlJob = palloc0(sizeof(DDLJob));
+	ddlJob->targetRelationId = relationId;
+	ddlJob->concurrentIndexCmd = false;
+	ddlJob->commandString = renameCommand;
+	ddlJob->taskList = DDLTaskList(relationId, renameCommand);
+
+	return list_make1(ddlJob);
+}
+
+
 /*
  * WorkerProcessAlterTableStmt checks and processes the alter table statement to be
  * worked on the distributed table of the worker node. Currently, it only processes
@@ -1980,40 +2032,24 @@ OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId)
 
 /*
  * ErrorIfDistributedRenameStmt errors out if the corresponding rename statement
- * operates on a distributed table or its objects.
+ * operates on any part of a distributed table other than a column.
  *
  * Note: This function handles only those rename statements which operate on tables.
  */
 static void
-ErrorIfDistributedRenameStmt(RenameStmt *renameStatement)
+ErrorIfUnsupportedRenameStmt(RenameStmt *renameStmt)
 {
-	Oid relationId = InvalidOid;
+	Assert(IsAlterTableRenameStmt(renameStmt));
-	bool isDistributedRelation = false;
 
-	Assert(IsAlterTableRenameStmt(renameStatement));
+	if (renameStmt->renameType == OBJECT_TABLE)
-
-	/*
-	 * The lock levels here should be same as the ones taken in
-	 * RenameRelation(), renameatt() and RenameConstraint(). However, since all
-	 * three statements have identical lock levels, we just use a single statement.
-	 */
-	relationId = RangeVarGetRelid(renameStatement->relation, AccessExclusiveLock,
-								  renameStatement->missing_ok);
-
-	/*
-	 * If the table does not exist, we don't do anything here, and allow postgres to
-	 * throw the appropriate error or notice message later.
-	 */
-	if (!OidIsValid(relationId))
-	{
-		return;
-	}
-
-	isDistributedRelation = IsDistributedTable(relationId);
-	if (isDistributedRelation)
 	{
 		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("renaming distributed tables or their objects is "
+						errmsg("renaming distributed tables is currently unsupported")));
+	}
+	else if (renameStmt->renameType == OBJECT_TABCONSTRAINT)
+	{
+		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("renaming constraints belonging to distributed tables is "
 							   "currently unsupported")));
 	}
 }
@@ -2099,11 +2135,12 @@ CreateLocalTable(RangeVar *relation, char *nodeName, int32 nodePort)
 
 
 /*
- * IsAlterTableRenameStmt returns true if the passed in RenameStmt operates on a
+ * IsAlterTableRenameStmt returns whether the passed-in RenameStmt is one of
- * distributed table or its objects. This includes:
+ * the following forms:
- * ALTER TABLE RENAME
+ *
- * ALTER TABLE RENAME COLUMN
+ *   - ALTER TABLE RENAME
- * ALTER TABLE RENAME CONSTRAINT
+ *   - ALTER TABLE RENAME COLUMN
+ *   - ALTER TABLE RENAME CONSTRAINT
  */
 static bool
 IsAlterTableRenameStmt(RenameStmt *renameStmt)

src/test/regress/expected/multi_name_lengths.out

@@ -125,9 +125,9 @@ DETAIL:  Citus cannot execute ADD CONSTRAINT command other than ADD CONSTRAINT F
 \c - - - :master_port
 -- Placeholders for RENAME operations
 ALTER TABLE name_lengths RENAME TO name_len_12345678901234567890123456789012345678901234567890;
-ERROR:  renaming distributed tables or their objects is currently unsupported
+ERROR:  renaming distributed tables is currently unsupported
 ALTER TABLE name_lengths RENAME CONSTRAINT unique_12345678901234567890123456789012345678901234567890 TO unique2_12345678901234567890123456789012345678901234567890;
-ERROR:  renaming distributed tables or their objects is currently unsupported
+ERROR:  renaming constraints belonging to distributed tables is currently unsupported
 -- Verify that CREATE INDEX on already distributed table has proper shard names.
 CREATE INDEX tmp_idx_12345678901234567890123456789012345678901234567890 ON name_lengths(col2);
 NOTICE:  using one-phase commit for distributed DDL commands

src/test/regress/expected/multi_reference_table.out

@@ -1359,7 +1359,7 @@ HINT:  You can enable two-phase commit for extra safety with: SET citus.multi_sh
 \c - - - :master_port
 -- as we expect, renaming and setting WITH OIDS does not work for reference tables
 ALTER TABLE reference_table_ddl RENAME TO reference_table_ddl_test;
-ERROR:  renaming distributed tables or their objects is currently unsupported
+ERROR:  renaming distributed tables is currently unsupported
 ALTER TABLE reference_table_ddl SET WITH OIDS;
 ERROR:  alter table command is currently unsupported
 DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT FOREIGN KEY and TYPE subcommands are supported.

src/test/regress/input/multi_alter_table_statements.source

@@ -98,6 +98,10 @@ ALTER TABLE lineitem_alter DROP COLUMN int_column1;
 ALTER TABLE lineitem_alter DROP COLUMN float_column;
 ALTER TABLE lineitem_alter DROP COLUMN date_column;
 
+-- Verify that RENAME COLUMN works
+ALTER TABLE lineitem_alter RENAME COLUMN l_orderkey TO l_orderkey_renamed;
+SELECT SUM(l_orderkey_renamed) FROM lineitem_alter;
+
 -- Verify that IF EXISTS works as expected
 
 ALTER TABLE non_existent_table ADD COLUMN new_column INTEGER;
@@ -107,6 +111,11 @@ ALTER TABLE IF EXISTS lineitem_alter ALTER COLUMN int_column2 SET DATA TYPE INTE
 ALTER TABLE lineitem_alter DROP COLUMN non_existent_column;
 ALTER TABLE lineitem_alter DROP COLUMN IF EXISTS non_existent_column;
 ALTER TABLE lineitem_alter DROP COLUMN IF EXISTS int_column2;
+
+-- Verify with IF EXISTS for extant table
+ALTER TABLE IF EXISTS lineitem_alter RENAME COLUMN l_orderkey_renamed TO l_orderkey;
+SELECT SUM(l_orderkey) FROM lineitem_alter;
+
 \d lineitem_alter
 
 -- Verify that we can execute commands with multiple subcommands
@@ -139,10 +148,9 @@ ALTER TABLE lineitem_alter ADD COLUMN new_column non_existent_type;
 ALTER TABLE lineitem_alter ALTER COLUMN null_column SET NOT NULL;
 ALTER TABLE lineitem_alter ALTER COLUMN l_partkey SET DEFAULT 'a';
 
--- Verify that we error out on statements involving RENAME
+-- Verify that we error out on non-column RENAME statements
 
 ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-ALTER TABLE lineitem_alter RENAME COLUMN l_orderkey TO l_orderkey_renamed;
 ALTER TABLE lineitem_alter RENAME CONSTRAINT constraint_a TO constraint_b;
 
 -- Verify that IF EXISTS works as expected with RENAME statements
@@ -150,7 +158,6 @@ ALTER TABLE lineitem_alter RENAME CONSTRAINT constraint_a TO constraint_b;
 ALTER TABLE non_existent_table RENAME TO non_existent_table_renamed;
 ALTER TABLE IF EXISTS non_existent_table RENAME TO non_existent_table_renamed;
 ALTER TABLE IF EXISTS non_existent_table RENAME COLUMN column1 TO column2;
-ALTER TABLE IF EXISTS lineitem_alter RENAME l_orderkey TO l_orderkey_renamed;
 
 -- Verify that none of the failed alter table commands took effect on the master
 -- node

src/test/regress/output/multi_alter_table_statements.source

@@ -253,6 +253,14 @@ SELECT int_column2, pg_typeof(int_column2), count(*) from lineitem_alter GROUP B
 ALTER TABLE lineitem_alter DROP COLUMN int_column1;
 ALTER TABLE lineitem_alter DROP COLUMN float_column;
 ALTER TABLE lineitem_alter DROP COLUMN date_column;
+-- Verify that RENAME COLUMN works
+ALTER TABLE lineitem_alter RENAME COLUMN l_orderkey TO l_orderkey_renamed;
+SELECT SUM(l_orderkey_renamed) FROM lineitem_alter;
+   sum    
+----------
+ 53620791
+(1 row)
+
 -- Verify that IF EXISTS works as expected
 ALTER TABLE non_existent_table ADD COLUMN new_column INTEGER;
 ERROR:  relation "non_existent_table" does not exist
@@ -264,6 +272,14 @@ ERROR:  column "non_existent_column" of relation "lineitem_alter" does not exist
 ALTER TABLE lineitem_alter DROP COLUMN IF EXISTS non_existent_column;
 NOTICE:  column "non_existent_column" of relation "lineitem_alter" does not exist, skipping
 ALTER TABLE lineitem_alter DROP COLUMN IF EXISTS int_column2;
+-- Verify with IF EXISTS for extant table
+ALTER TABLE IF EXISTS lineitem_alter RENAME COLUMN l_orderkey_renamed TO l_orderkey;
+SELECT SUM(l_orderkey) FROM lineitem_alter;
+   sum    
+----------
+ 53620791
+(1 row)
+
 \d lineitem_alter
             Table "public.lineitem_alter"
      Column      |         Type          | Modifiers 
@@ -365,13 +381,11 @@ ERROR:  column "null_column" contains null values
 CONTEXT:  while executing command on localhost:57638
 ALTER TABLE lineitem_alter ALTER COLUMN l_partkey SET DEFAULT 'a';
 ERROR:  invalid input syntax for integer: "a"
--- Verify that we error out on statements involving RENAME
+-- Verify that we error out on non-column RENAME statements
 ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-ERROR:  renaming distributed tables or their objects is currently unsupported
+ERROR:  renaming distributed tables is currently unsupported
-ALTER TABLE lineitem_alter RENAME COLUMN l_orderkey TO l_orderkey_renamed;
-ERROR:  renaming distributed tables or their objects is currently unsupported
 ALTER TABLE lineitem_alter RENAME CONSTRAINT constraint_a TO constraint_b;
-ERROR:  renaming distributed tables or their objects is currently unsupported
+ERROR:  renaming constraints belonging to distributed tables is currently unsupported
 -- Verify that IF EXISTS works as expected with RENAME statements
 ALTER TABLE non_existent_table RENAME TO non_existent_table_renamed;
 ERROR:  relation "non_existent_table" does not exist
@@ -379,8 +393,6 @@ ALTER TABLE IF EXISTS non_existent_table RENAME TO non_existent_table_renamed;
 NOTICE:  relation "non_existent_table" does not exist, skipping
 ALTER TABLE IF EXISTS non_existent_table RENAME COLUMN column1 TO column2;
 NOTICE:  relation "non_existent_table" does not exist, skipping
-ALTER TABLE IF EXISTS lineitem_alter RENAME l_orderkey TO l_orderkey_renamed;
-ERROR:  renaming distributed tables or their objects is currently unsupported
 -- Verify that none of the failed alter table commands took effect on the master
 -- node
 \d lineitem_alter