Merge pull request #1171 from citusdata/utility_schemanode_fix

Remove call to SchemaNode() in multi_ProcessUtility

src/backend/distributed/executor/multi_utility.c

@@ -170,11 +170,24 @@ multi_ProcessUtility(Node *parsetree,
 					 DestReceiver *dest,
 					 char *completionTag)
 {
-	bool isCoordinator = IsCoordinator();
 	bool commandMustRunAsOwner = false;
 	Oid savedUserId = InvalidOid;
 	int savedSecurityContext = 0;
 
+	if (IsA(parsetree, TransactionStmt))
+	{
+		/*
+		 * Transaction statements (e.g. ABORT, COMMIT) can be run in aborted
+		 * transactions in which case a lot of checks cannot be done safely in
+		 * that state. Since we never need to intercept transaction statements,
+		 * skip our checks and immediately fall into standard_ProcessUtility.
+		 */
+		standard_ProcessUtility(parsetree, queryString, context,
+								params, dest, completionTag);
+
+		return;
+	}
+
 	/*
 	 * TRANSMIT used to be separate command, but to avoid patching the grammar
 	 * it's no overlaid onto COPY, but with FORMAT = 'transmit' instead of the
@@ -296,19 +309,30 @@ multi_ProcessUtility(Node *parsetree,
 									  "move all tables.")));
 		}
 	}
-	else if (!isCoordinator)
+	else
 	{
+		/*
+		 * 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
+		 * 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.
+		 */
+
 		if (IsA(parsetree, AlterTableStmt))
 		{
 			AlterTableStmt *alterTableStmt = (AlterTableStmt *) parsetree;
 			if (alterTableStmt->relkind == OBJECT_TABLE)
 			{
 				/*
-				 * When the coordinator issues an ALTER TABLE ... ADD FOREIGN KEY
+				 * When issuing an ALTER TABLE ... ADD FOREIGN KEY command, the
-				 * command, the validation step should be skipped on the distributed
+				 * the validation step should be skipped on the distributed table.
-				 * table of the worker. Therefore, we check whether the given ALTER
+				 * Therefore, we check whether the given ALTER TABLE statement is a
-				 * TABLE statement is a FOREIGN KEY constraint and if so disable the
+				 * FOREIGN KEY constraint and if so disable the validation step.
-				 * validation step. Note that validation is done on the shard level.
+				 * Note that validation is done on the shard level when DDL
+				 * propagation is enabled.
 				 */
 				parsetree = WorkerProcessAlterTableStmt(alterTableStmt, queryString);
 			}

src/test/regress/expected/multi_foreign_key.out

@@ -407,6 +407,13 @@ SELECT master_create_worker_shards('referencing_table', 4, 1);
  
 (1 row)
 
+-- verify that we skip foreign key validation when propagation is turned off
+-- not skipping validation would result in a distributed query, which emits debug messages
+BEGIN;
+SET LOCAL citus.enable_ddl_propagation TO off;
+SET LOCAL client_min_messages TO DEBUG2;
+ALTER TABLE referencing_table ADD CONSTRAINT test_constraint FOREIGN KEY (ref_id) REFERENCES referenced_table (id);
+ABORT;
 -- test foreign constraint creation
 -- test foreign constraint creation with not supported parameters
 ALTER TABLE referencing_table ADD CONSTRAINT test_constraint FOREIGN KEY(ref_id) REFERENCES referenced_table(id) ON DELETE SET NULL;

src/test/regress/sql/multi_foreign_key.sql

@@ -220,6 +220,13 @@ CREATE TABLE referencing_table(id int, ref_id int);
 SELECT master_create_distributed_table('referencing_table', 'ref_id', 'hash');
 SELECT master_create_worker_shards('referencing_table', 4, 1);
 
+-- verify that we skip foreign key validation when propagation is turned off
+-- not skipping validation would result in a distributed query, which emits debug messages
+BEGIN;
+SET LOCAL citus.enable_ddl_propagation TO off;
+SET LOCAL client_min_messages TO DEBUG2;
+ALTER TABLE referencing_table ADD CONSTRAINT test_constraint FOREIGN KEY (ref_id) REFERENCES referenced_table (id);
+ABORT;
 
 -- test foreign constraint creation
 -- test foreign constraint creation with not supported parameters