Error out for unsupported trigger objects

Error out if creating a citus table from a table having triggers.
Error out for CREATE TRIGGER commands that are run on citus tables.

src/backend/distributed/commands/create_distributed_table.c

@@ -99,6 +99,7 @@ static void EnsureTableCanBeColocatedWith(Oid relationId, char replicationModel,
 										  Oid sourceRelationId);
 static void EnsureLocalTableEmpty(Oid relationId);
 static void EnsureTableNotDistributed(Oid relationId);
+static void EnsureRelationHasNoTriggers(Oid relationId);
 static Oid SupportFunctionForColumn(Var *partitionColumn, Oid accessMethodId,
 									int16 supportFunctionNumber);
 static void EnsureLocalTableEmptyIfNecessary(Oid relationId, char distributionMethod,
@@ -650,6 +651,7 @@ EnsureRelationCanBeDistributed(Oid relationId, Var *distributionColumn,
 	EnsureTableNotDistributed(relationId);
 	EnsureLocalTableEmptyIfNecessary(relationId, distributionMethod, viaDeprecatedAPI);
 	EnsureReplicationSettings(InvalidOid, replicationModel);
+	EnsureRelationHasNoTriggers(relationId);
 
 	/* we assume callers took necessary locks */
 	Relation relation = relation_open(relationId, NoLock);
@@ -972,6 +974,31 @@ EnsureReplicationSettings(Oid relationId, char replicationModel)
 }
 
 
+/*
+ * EnsureRelationHasNoTriggers errors out if the given table has triggers on
+ * it. See also GetExplicitTriggerIdList function's comment for the triggers this
+ * function errors out.
+ */
+static void
+EnsureRelationHasNoTriggers(Oid relationId)
+{
+	List *explicitTriggerIds = GetExplicitTriggerIdList(relationId);
+
+	if (list_length(explicitTriggerIds) > 0)
+	{
+		char *relationName = get_rel_name(relationId);
+
+		Assert(relationName != NULL);
+		ereport(ERROR, (errmsg("cannot distribute relation \"%s\" because it has "
+							   "triggers ", relationName),
+						errdetail("Citus does not support distributing tables with "
+								  "triggers."),
+						errhint("Drop all the triggers on \"%s\" and retry.",
+								relationName)));
+	}
+}
+
+
 /*
  * LookupDistributionMethod maps the oids of citus.distribution_type enum
  * values to pg_dist_partition.partmethod values.
@@ -1176,7 +1203,7 @@ CreateTruncateTrigger(Oid relationId)
 	CreateTrigStmt *trigger = makeNode(CreateTrigStmt);
 	trigger->trigname = triggerName->data;
 	trigger->relation = NULL;
-	trigger->funcname = SystemFuncName("citus_truncate_trigger");
+	trigger->funcname = SystemFuncName(CITUS_TRUNCATE_TRIGGER_NAME);
 	trigger->args = NIL;
 	trigger->row = false;
 	trigger->timing = TRIGGER_TYPE_AFTER;

src/backend/distributed/commands/trigger.c

@@ -139,3 +139,48 @@ get_relation_trigger_oid_compat(HeapTuple heapTuple)
 
 	return triggerOid;
 }
+
+
+/*
+ * ErrorIfUnsupportedCreateTriggerCommand errors out for the CREATE TRIGGER
+ * command that is run for a citus table if it is not citus_truncate_trigger.
+ *
+ * Note that internal triggers that are created implicitly by postgres for
+ * foreign key validation already wouldn't be executed via process utility,
+ * hence there is no need to check that case here.
+ */
+void
+ErrorIfUnsupportedCreateTriggerCommand(CreateTrigStmt *createTriggerStmt)
+{
+	RangeVar *triggerRelation = createTriggerStmt->relation;
+
+	bool missingOk = true;
+	Oid relationId = RangeVarGetRelid(triggerRelation, AccessShareLock, missingOk);
+
+	if (!OidIsValid(relationId))
+	{
+		/*
+		 * standard_ProcessUtility would already error out if the given table
+		 * does not exist
+		 */
+		return;
+	}
+
+	if (!IsCitusTable(relationId))
+	{
+		return;
+	}
+
+	char *functionName = makeRangeVarFromNameList(createTriggerStmt->funcname)->relname;
+	if (strncmp(functionName, CITUS_TRUNCATE_TRIGGER_NAME, NAMEDATALEN) == 0)
+	{
+		return;
+	}
+
+	char *relationName = triggerRelation->relname;
+
+	Assert(relationName != NULL);
+	ereport(ERROR, (errmsg("cannot create trigger on relation \"%s\" because it "
+						   "is either a distributed table or a reference table",
+						   relationName)));
+}

src/backend/distributed/commands/utility_hook.c

@@ -208,6 +208,13 @@ multi_ProcessUtility(PlannedStmt *pstmt,
 		parsetree = ProcessCreateSubscriptionStmt(createSubStmt);
 	}
 
+	if (IsA(parsetree, CreateTrigStmt))
+	{
+		CreateTrigStmt *createTriggerStmt = (CreateTrigStmt *) parsetree;
+
+		ErrorIfUnsupportedCreateTriggerCommand(createTriggerStmt);
+	}
+
 	if (IsA(parsetree, CallStmt))
 	{
 		CallStmt *callStmt = (CallStmt *) parsetree;

src/include/distributed/commands.h

@@ -44,6 +44,8 @@ typedef struct DistributeObjectOps
 	ObjectAddress (*address)(Node *, bool);
 } DistributeObjectOps;
 
+#define CITUS_TRUNCATE_TRIGGER_NAME "citus_truncate_trigger"
+
 const DistributeObjectOps * GetDistributeObjectOps(Node *node);
 
 /* cluster.c - forward declarations */
@@ -278,6 +280,7 @@ extern void PostprocessVacuumStmt(VacuumStmt *vacuumStmt, const char *vacuumComm
 extern List * GetExplicitTriggerCommandList(Oid relationId);
 extern List * GetExplicitTriggerIdList(Oid relationId);
 extern Oid get_relation_trigger_oid_compat(HeapTuple heapTuple);
+extern void ErrorIfUnsupportedCreateTriggerCommand(CreateTrigStmt *createTriggerStmt);
 
 extern bool ShouldPropagateSetCommand(VariableSetStmt *setStmt);
 extern void PostprocessVariableSetStmt(VariableSetStmt *setStmt, const char *setCommand);

src/test/regress/expected/create_table_triggers.out

@@ -0,0 +1,101 @@
+-- This test file includes tests to show that we do not allow triggers
+-- on citus tables. Note that in other regression tests, we already test
+-- the successfull citus table creation cases.
+\set VERBOSITY terse
+SET citus.next_shard_id TO 1505000;
+CREATE SCHEMA table_triggers_schema;
+SET search_path TO table_triggers_schema;
+---------------------------------------------------------------------
+-- show that we do not allow trigger creation on citus tables
+---------------------------------------------------------------------
+-- create a simple function to be invoked by triggers
+CREATE FUNCTION update_value() RETURNS trigger AS $update_value$
+BEGIN
+    NEW.value := value+1 ;
+    RETURN NEW;
+END;
+$update_value$ LANGUAGE plpgsql;
+CREATE TABLE distributed_table (value int);
+SELECT create_distributed_table('distributed_table', 'value');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+CREATE TABLE reference_table (value int);
+SELECT create_reference_table('reference_table');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+-- below two should fail
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+ERROR:  cannot create trigger on relation "distributed_table" because it is either a distributed table or a reference table
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+ERROR:  cannot create trigger on relation "reference_table" because it is either a distributed table or a reference table
+---------------------------------------------------------------------
+-- show that we do not allow creating citus tables if the
+-- table has already triggers
+---------------------------------------------------------------------
+CREATE TABLE distributed_table_1 (value int);
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+CREATE TABLE reference_table_1 (value int);
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+-- below two should fail
+SELECT create_distributed_table('distributed_table_1', 'value');
+ERROR:  cannot distribute relation "distributed_table_1" because it has triggers
+SELECT create_reference_table('reference_table_1');
+ERROR:  cannot distribute relation "reference_table_1" because it has triggers
+---------------------------------------------------------------------
+-- test deparse logic for CREATE TRIGGER commands
+-- via master_get_table_ddl_events
+---------------------------------------------------------------------
+CREATE TABLE test_table (
+    id int,
+    text_number text,
+    text_col text
+);
+CREATE FUNCTION test_table_trigger_function() RETURNS trigger AS $test_table_trigger_function$
+BEGIN
+    RAISE EXCEPTION 'a meaningless exception';
+END;
+$test_table_trigger_function$ LANGUAGE plpgsql;
+-- in below two, use constraint triggers to test DEFERRABLE | NOT DEFERRABLE syntax
+CREATE CONSTRAINT TRIGGER test_table_update
+    AFTER UPDATE OF id ON test_table
+    NOT DEFERRABLE
+    FOR EACH ROW
+    WHEN (OLD.* IS NOT DISTINCT FROM NEW.* AND OLD.text_number IS NOT NULL)
+    EXECUTE FUNCTION test_table_trigger_function();
+CREATE CONSTRAINT TRIGGER test_table_insert
+    AFTER INSERT ON test_table
+    DEFERRABLE INITIALLY IMMEDIATE
+    FOR EACH ROW
+    WHEN (NEW.id > 5 OR NEW.text_col IS NOT NULL AND NEW.id < to_number(NEW.text_number, '9999'))
+    EXECUTE FUNCTION test_table_trigger_function();
+CREATE TRIGGER test_table_delete
+    AFTER DELETE ON test_table
+    FOR EACH STATEMENT
+    EXECUTE FUNCTION test_table_trigger_function();
+SELECT master_get_table_ddl_events('test_table');
+                                                                                                                                                                           master_get_table_ddl_events
+---------------------------------------------------------------------
+ CREATE TABLE table_triggers_schema.test_table (id integer, text_number text, text_col text)
+ ALTER TABLE table_triggers_schema.test_table OWNER TO postgres
+ CREATE TRIGGER test_table_delete AFTER DELETE ON table_triggers_schema.test_table FOR EACH STATEMENT EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
+ CREATE CONSTRAINT TRIGGER test_table_insert AFTER INSERT ON table_triggers_schema.test_table DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((new.id OPERATOR(pg_catalog.>) 5) OR ((new.text_col IS NOT NULL) AND ((new.id)::numeric OPERATOR(pg_catalog.<) to_number(new.text_number, '9999'::text))))) EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
+ CREATE CONSTRAINT TRIGGER test_table_update AFTER UPDATE OF id ON table_triggers_schema.test_table NOT DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((NOT (old.* IS DISTINCT FROM new.*)) AND (old.text_number IS NOT NULL))) EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
+(5 rows)
+
+-- cleanup at exit
+DROP SCHEMA table_triggers_schema CASCADE;
+NOTICE:  drop cascades to 7 other objects

src/test/regress/expected/create_table_triggers_0.out

@@ -0,0 +1,101 @@
+-- This test file includes tests to show that we do not allow triggers
+-- on citus tables. Note that in other regression tests, we already test
+-- the successfull citus table creation cases.
+\set VERBOSITY terse
+SET citus.next_shard_id TO 1505000;
+CREATE SCHEMA table_triggers_schema;
+SET search_path TO table_triggers_schema;
+---------------------------------------------------------------------
+-- show that we do not allow trigger creation on citus tables
+---------------------------------------------------------------------
+-- create a simple function to be invoked by triggers
+CREATE FUNCTION update_value() RETURNS trigger AS $update_value$
+BEGIN
+    NEW.value := value+1 ;
+    RETURN NEW;
+END;
+$update_value$ LANGUAGE plpgsql;
+CREATE TABLE distributed_table (value int);
+SELECT create_distributed_table('distributed_table', 'value');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+CREATE TABLE reference_table (value int);
+SELECT create_reference_table('reference_table');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+-- below two should fail
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+ERROR:  cannot create trigger on relation "distributed_table" because it is either a distributed table or a reference table
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+ERROR:  cannot create trigger on relation "reference_table" because it is either a distributed table or a reference table
+---------------------------------------------------------------------
+-- show that we do not allow creating citus tables if the
+-- table has already triggers
+---------------------------------------------------------------------
+CREATE TABLE distributed_table_1 (value int);
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+CREATE TABLE reference_table_1 (value int);
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+-- below two should fail
+SELECT create_distributed_table('distributed_table_1', 'value');
+ERROR:  cannot distribute relation "distributed_table_1" because it has triggers
+SELECT create_reference_table('reference_table_1');
+ERROR:  cannot distribute relation "reference_table_1" because it has triggers
+---------------------------------------------------------------------
+-- test deparse logic for CREATE TRIGGER commands
+-- via master_get_table_ddl_events
+---------------------------------------------------------------------
+CREATE TABLE test_table (
+    id int,
+    text_number text,
+    text_col text
+);
+CREATE FUNCTION test_table_trigger_function() RETURNS trigger AS $test_table_trigger_function$
+BEGIN
+    RAISE EXCEPTION 'a meaningless exception';
+END;
+$test_table_trigger_function$ LANGUAGE plpgsql;
+-- in below two, use constraint triggers to test DEFERRABLE | NOT DEFERRABLE syntax
+CREATE CONSTRAINT TRIGGER test_table_update
+    AFTER UPDATE OF id ON test_table
+    NOT DEFERRABLE
+    FOR EACH ROW
+    WHEN (OLD.* IS NOT DISTINCT FROM NEW.* AND OLD.text_number IS NOT NULL)
+    EXECUTE FUNCTION test_table_trigger_function();
+CREATE CONSTRAINT TRIGGER test_table_insert
+    AFTER INSERT ON test_table
+    DEFERRABLE INITIALLY IMMEDIATE
+    FOR EACH ROW
+    WHEN (NEW.id > 5 OR NEW.text_col IS NOT NULL AND NEW.id < to_number(NEW.text_number, '9999'))
+    EXECUTE FUNCTION test_table_trigger_function();
+CREATE TRIGGER test_table_delete
+    AFTER DELETE ON test_table
+    FOR EACH STATEMENT
+    EXECUTE FUNCTION test_table_trigger_function();
+SELECT master_get_table_ddl_events('test_table');
+                                                                                                                                                                            master_get_table_ddl_events
+---------------------------------------------------------------------
+ CREATE TABLE table_triggers_schema.test_table (id integer, text_number text, text_col text)
+ ALTER TABLE table_triggers_schema.test_table OWNER TO postgres
+ CREATE TRIGGER test_table_delete AFTER DELETE ON table_triggers_schema.test_table FOR EACH STATEMENT EXECUTE PROCEDURE table_triggers_schema.test_table_trigger_function()
+ CREATE CONSTRAINT TRIGGER test_table_insert AFTER INSERT ON table_triggers_schema.test_table DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((new.id OPERATOR(pg_catalog.>) 5) OR ((new.text_col IS NOT NULL) AND ((new.id)::numeric OPERATOR(pg_catalog.<) to_number(new.text_number, '9999'::text))))) EXECUTE PROCEDURE table_triggers_schema.test_table_trigger_function()
+ CREATE CONSTRAINT TRIGGER test_table_update AFTER UPDATE OF id ON table_triggers_schema.test_table NOT DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((NOT (old.* IS DISTINCT FROM new.*)) AND (old.text_number IS NOT NULL))) EXECUTE PROCEDURE table_triggers_schema.test_table_trigger_function()
+(5 rows)
+
+-- cleanup at exit
+DROP SCHEMA table_triggers_schema CASCADE;
+NOTICE:  drop cascades to 7 other objects

src/test/regress/multi_schedule

@@ -45,7 +45,7 @@ test: multi_create_table_constraints multi_master_protocol multi_load_data multi
 test: multi_behavioral_analytics_basics multi_behavioral_analytics_single_shard_queries multi_insert_select_non_pushable_queries multi_insert_select multi_behavioral_analytics_create_table_superuser
 test: multi_shard_update_delete recursive_dml_with_different_planners_executors
 test: insert_select_repartition window_functions dml_recursive multi_insert_select_window
-test: multi_insert_select_conflict
+test: multi_insert_select_conflict create_table_triggers
 test: multi_row_insert
 
 # following should not run in parallel because it relies on connection counts to workers

src/test/regress/sql/create_table_triggers.sql

@@ -0,0 +1,100 @@
+-- This test file includes tests to show that we do not allow triggers
+-- on citus tables. Note that in other regression tests, we already test
+-- the successfull citus table creation cases.
+
+\set VERBOSITY terse
+
+SET citus.next_shard_id TO 1505000;
+
+CREATE SCHEMA table_triggers_schema;
+SET search_path TO table_triggers_schema;
+
+-------------------------------------------------------------
+-- show that we do not allow trigger creation on citus tables
+-------------------------------------------------------------
+
+-- create a simple function to be invoked by triggers
+CREATE FUNCTION update_value() RETURNS trigger AS $update_value$
+BEGIN
+    NEW.value := value+1 ;
+    RETURN NEW;
+END;
+$update_value$ LANGUAGE plpgsql;
+
+CREATE TABLE distributed_table (value int);
+SELECT create_distributed_table('distributed_table', 'value');
+
+CREATE TABLE reference_table (value int);
+SELECT create_reference_table('reference_table');
+
+-- below two should fail
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+
+---------------------------------------------------------
+-- show that we do not allow creating citus tables if the
+-- table has already triggers
+---------------------------------------------------------
+
+CREATE TABLE distributed_table_1 (value int);
+
+CREATE TRIGGER update_value_dist
+AFTER INSERT ON distributed_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+
+CREATE TABLE reference_table_1 (value int);
+
+CREATE TRIGGER update_value_ref
+AFTER INSERT ON reference_table_1
+FOR EACH ROW EXECUTE PROCEDURE update_value();
+
+-- below two should fail
+SELECT create_distributed_table('distributed_table_1', 'value');
+SELECT create_reference_table('reference_table_1');
+
+-------------------------------------------------
+-- test deparse logic for CREATE TRIGGER commands
+-- via master_get_table_ddl_events
+-------------------------------------------------
+
+CREATE TABLE test_table (
+    id int,
+    text_number text,
+    text_col text
+);
+
+CREATE FUNCTION test_table_trigger_function() RETURNS trigger AS $test_table_trigger_function$
+BEGIN
+    RAISE EXCEPTION 'a meaningless exception';
+END;
+$test_table_trigger_function$ LANGUAGE plpgsql;
+
+-- in below two, use constraint triggers to test DEFERRABLE | NOT DEFERRABLE syntax
+CREATE CONSTRAINT TRIGGER test_table_update
+    AFTER UPDATE OF id ON test_table
+    NOT DEFERRABLE
+    FOR EACH ROW
+    WHEN (OLD.* IS NOT DISTINCT FROM NEW.* AND OLD.text_number IS NOT NULL)
+    EXECUTE FUNCTION test_table_trigger_function();
+
+CREATE CONSTRAINT TRIGGER test_table_insert
+    AFTER INSERT ON test_table
+    DEFERRABLE INITIALLY IMMEDIATE
+    FOR EACH ROW
+    WHEN (NEW.id > 5 OR NEW.text_col IS NOT NULL AND NEW.id < to_number(NEW.text_number, '9999'))
+    EXECUTE FUNCTION test_table_trigger_function();
+
+CREATE TRIGGER test_table_delete
+    AFTER DELETE ON test_table
+    FOR EACH STATEMENT
+    EXECUTE FUNCTION test_table_trigger_function();
+
+SELECT master_get_table_ddl_events('test_table');
+
+-- cleanup at exit
+DROP SCHEMA table_triggers_schema CASCADE;