EnterpriseDB · francoisp · Sep 22, 2020
diff --git a/expected/dml.out b/expected/dml.out
@@ -14,7 +14,7 @@ CREATE USER MAPPING FOR public SERVER mysql_svr
 -- Create foreign tables
 CREATE FOREIGN TABLE f_mysql_test(a int, b int)
   SERVER mysql_svr OPTIONS (dbname 'mysql_fdw_regress', table_name 'mysql_test');
-CREATE FOREIGN TABLE fdw126_ft1(stu_id int, stu_name varchar(255))
+CREATE FOREIGN TABLE fdw126_ft1(stu_id int, stu_name varchar(255), stu_dept int)
   SERVER mysql_svr OPTIONS (dbname 'mysql_fdw_regress1', table_name 'student');
 CREATE FOREIGN TABLE fdw126_ft2(stu_id int, stu_name varchar(255))
   SERVER mysql_svr OPTIONS (table_name 'student');
@@ -56,7 +56,7 @@ SELECT emp_id, emp_dat FROM f_empdata ORDER BY 1;
 -- the operation on foreign table created for tables in mysql_fdw_regress
 -- MySQL database.  Below operations will be performed for foreign table
 -- created for table in mysql_fdw_regress1 MySQL database.
-INSERT INTO fdw126_ft1 VALUES(1, 'One');
+INSERT INTO fdw126_ft1 VALUES(1, 'One', 101);
 UPDATE fdw126_ft1 SET stu_name = 'one' WHERE stu_id = 1;
 DELETE FROM fdw126_ft1 WHERE stu_id = 1;
 -- Select on f_mysql_test foreign table which is created for mysql_test table
@@ -116,6 +116,50 @@ ANALYZE f_empdata(emp_id);
 WARNING:  skipping "f_empdata" --- cannot analyze this foreign table
 VACUUM ANALYZE f_empdata;
 WARNING:  skipping "f_empdata" --- cannot vacuum non-tables or special system tables
+-- Verify the before update trigger which modifies the column value which is not
+-- part of update statement.
+CREATE FUNCTION before_row_update_func() RETURNS TRIGGER AS $$
+BEGIN
+  NEW.stu_name := NEW.stu_name || ' trigger updated!';
+	RETURN NEW;
+  END
+$$ language plpgsql;
+CREATE TRIGGER before_row_update_trig
+BEFORE UPDATE ON fdw126_ft1
+FOR EACH ROW EXECUTE PROCEDURE before_row_update_func();
+INSERT INTO fdw126_ft1 VALUES(1, 'One', 101);
+EXPLAIN (verbose, costs off)
+UPDATE fdw126_ft1 SET stu_dept = 201 WHERE stu_id = 1;
+                                                             QUERY PLAN                                                              
+-------------------------------------------------------------------------------------------------------------------------------------
+ Update on public.fdw126_ft1
+   ->  Foreign Scan on public.fdw126_ft1
+         Output: stu_id, stu_name, 201, stu_id, fdw126_ft1.*
+         Local server startup cost: 10
+         Remote query: SELECT `stu_id`, `stu_name`, `stu_dept` FROM `mysql_fdw_regress1`.`student` WHERE ((`stu_id` = 1)) FOR UPDATE
+(5 rows)
+
+UPDATE fdw126_ft1 SET stu_dept = 201 WHERE stu_id = 1;
+SELECT * FROM fdw126_ft1 ORDER BY stu_id;
+ stu_id |       stu_name       | stu_dept 
+--------+----------------------+----------
+      1 | One trigger updated! |      201
+(1 row)
+
+-- Throw an error when target list has row identifier column.
+UPDATE fdw126_ft1 SET stu_dept = 201, stu_id = 10  WHERE stu_id = 1;
+ERROR:  row identifier column update is not supported
+-- Throw an error when before row update trigger modify the row identifier
+-- column value.
+CREATE OR REPLACE FUNCTION before_row_update_func() RETURNS TRIGGER AS $$
+BEGIN
+  NEW.stu_name := NEW.stu_name || ' trigger updated!';
+  NEW.stu_id = 20;
+	RETURN NEW;
+  END
+$$ language plpgsql;
+UPDATE fdw126_ft1 SET stu_dept = 301 WHERE stu_id = 1;
+ERROR:  row identifier column update through trigger is not supported
 -- Cleanup
 DELETE FROM fdw126_ft1;
 DELETE FROM f_empdata;
@@ -127,6 +171,7 @@ DROP FOREIGN TABLE fdw126_ft4;
 DROP FOREIGN TABLE fdw126_ft5;
 DROP FOREIGN TABLE fdw126_ft6;
 DROP FOREIGN TABLE f_empdata;
+DROP FUNCTION before_row_update_func();
 DROP USER MAPPING FOR public SERVER mysql_svr;
 DROP SERVER mysql_svr;
 DROP EXTENSION mysql_fdw;
diff --git a/mysql_fdw.c b/mysql_fdw.c
@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "access/reloptions.h"
 #if PG_VERSION_NUM >= 120000
@@ -48,9 +49,11 @@
 #include "parser/parsetree.h"
 #include "storage/ipc.h"
 #include "utils/builtins.h"
+#include "utils/datum.h"
 #include "utils/guc.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
+#include "utils/syscache.h"
 
 /* Declarations for dynamic loading */
 PG_MODULE_MAGIC;
@@ -1193,11 +1196,44 @@ mysqlPlanForeignModify(PlannerInfo *root,
 				(errcode(ERRCODE_FDW_UNABLE_TO_CREATE_EXECUTION),
 				 errmsg("first column of remote table must be unique for INSERT/UPDATE/DELETE operation")));
 
-	if (operation == CMD_INSERT)
+	/*
+	 * In an INSERT, we transmit all columns that are defined in the foreign
+	 * table.  In an UPDATE, if there are BEFORE ROW UPDATE triggers on the
+	 * foreign table, we transmit all columns like INSERT; else we transmit
+	 * only columns that were explicitly targets of the UPDATE, so as to avoid
+	 * unnecessary data transmission.  (We can't do that for INSERT since we
+	 * would miss sending default values for columns not listed in the source
+	 * statement, and for UPDATE if there are BEFORE ROW UPDATE triggers since
+	 * those triggers might change values for non-target columns, in which
+	 * case we would miss sending changed values for those columns.)
+	 */
+	if (operation == CMD_INSERT ||
+		(operation == CMD_UPDATE &&
+		 rel->trigdesc &&
+		 rel->trigdesc->trig_update_before_row))
 	{
 		TupleDesc	tupdesc = RelationGetDescr(rel);
 		int			attnum;
 
+		if (operation == CMD_UPDATE)
+		{
+			Bitmapset  *tmpset = bms_copy(rte->updatedCols);
+			AttrNumber	col;
+
+			while ((col = bms_first_member(tmpset)) >= 0)
+			{
+				col += FirstLowInvalidHeapAttributeNumber;
+				if (col <= InvalidAttrNumber)	/* shouldn't happen */
+					elog(ERROR, "system-column update is not supported");
+
+				/*
+				 * We also disallow updates to the first column
+				 */
+				if (col == 1)		/* shouldn't happen */
+					elog(ERROR, "row identifier column update is not supported");
+			}
+		}
+
 		for (attnum = 1; attnum <= tupdesc->natts; attnum++)
 		{
 			Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
@@ -1444,6 +1480,9 @@ mysqlExecForeignUpdate(EState *estate,
 	Datum		value;
 	int			n_params;
 	bool	   *isnull;
+	Datum		new_value = 0;
+	HeapTuple	tp;
+	Form_pg_attribute att_tup;
 
 	n_params = list_length(fmstate->retrieved_attrs);
 
@@ -1456,9 +1495,16 @@ mysqlExecForeignUpdate(EState *estate,
 		int			attnum = lfirst_int(lc);
 		Oid			type;
 
-		/* first attribute cannot be in target list attribute */
+		/*
+		 * First attribute cannot be in target list attribute. Store the
+		 * modified row identifier column value so that we can compare it later
+		 * to check if that value has been changed through trigger.
+		 */
 		if (attnum == 1)
+		{
+			new_value = slot_getattr(slot, attnum, (bool *) (&isnull[bindnum]));
 			continue;
+		}
 
 		type = TupleDescAttr(slot->tts_tupleDescriptor, attnum - 1)->atttypid;
 		value = slot_getattr(slot, attnum, (bool *) (&isnull[bindnum]));
@@ -1468,9 +1514,29 @@ mysqlExecForeignUpdate(EState *estate,
 		bindnum++;
 	}
 
-	/* Get the id that was passed up as a resjunk column */
+	/*
+	 * Get the row identifier column value that was passed up as a resjunk
+	 * column and compare that value with the new value to identify if that
+	 * value is changed by trigger.
+	 */
 	value = ExecGetJunkAttribute(planSlot, 1, &is_null);
-	typeoid = get_atttype(foreignTableId, 1);
+
+	tp = SearchSysCache2(ATTNUM,
+						 ObjectIdGetDatum(foreignTableId),
+						 Int16GetDatum(1));
+
+	if (!HeapTupleIsValid(tp))
+		elog(ERROR, "cache lookup failed for attribute %d of relation %u",
+			 1, foreignTableId);
+
+	att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+
+	typeoid = att_tup->atttypid;
+
+	if (!datumIsEqual(value, new_value, att_tup->attbyval, att_tup->attlen))
+		elog(ERROR, "row identifier column update through trigger is not supported");
+
+	ReleaseSysCache(tp);
 
 	/* Bind qual */
 	mysql_bind_sql_var(typeoid, bindnum, value, mysql_bind_buffer, &is_null);

diff --git a/mysql_init.sh b/mysql_init.sh
@@ -36,7 +36,7 @@ mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e "CREATE TABLE numbers (a int PRIMARY KEY, b varchar(255));"
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e "CREATE TABLE test_tbl1 (c1 INT primary key, c2 VARCHAR(10), c3 CHAR(9), c4 MEDIUMINT, c5 DATE, c6 DECIMAL(10,5), c7 INT, c8 SMALLINT);"
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e "CREATE TABLE test_tbl2 (c1 INT primary key, c2 TEXT, c3 TEXT);"
-mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress1 -e "CREATE TABLE student (stu_id int PRIMARY KEY, stu_name text);"
+mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress1 -e "CREATE TABLE student (stu_id int PRIMARY KEY, stu_name text, stu_dept int);"
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress1 -e "CREATE TABLE numbers (a int, b varchar(255));"
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e "CREATE TABLE enum_t1 (id int PRIMARY KEY, size ENUM('small', 'medium', 'large'));"
 mysql -h $MYSQL_HOST -u $MYSQL_USER_NAME -P $MYSQL_PORT -D mysql_fdw_regress -e "INSERT INTO enum_t1 VALUES (1, 'small'),(2, 'medium'),(3, 'medium');"
diff --git a/sql/dml.sql b/sql/dml.sql
@@ -17,7 +17,7 @@ CREATE USER MAPPING FOR public SERVER mysql_svr
 -- Create foreign tables
 CREATE FOREIGN TABLE f_mysql_test(a int, b int)
   SERVER mysql_svr OPTIONS (dbname 'mysql_fdw_regress', table_name 'mysql_test');
-CREATE FOREIGN TABLE fdw126_ft1(stu_id int, stu_name varchar(255))
+CREATE FOREIGN TABLE fdw126_ft1(stu_id int, stu_name varchar(255), stu_dept int)
   SERVER mysql_svr OPTIONS (dbname 'mysql_fdw_regress1', table_name 'student');
 CREATE FOREIGN TABLE fdw126_ft2(stu_id int, stu_name varchar(255))
   SERVER mysql_svr OPTIONS (table_name 'student');
@@ -48,7 +48,7 @@ SELECT emp_id, emp_dat FROM f_empdata ORDER BY 1;
 -- the operation on foreign table created for tables in mysql_fdw_regress
 -- MySQL database.  Below operations will be performed for foreign table
 -- created for table in mysql_fdw_regress1 MySQL database.
-INSERT INTO fdw126_ft1 VALUES(1, 'One');
+INSERT INTO fdw126_ft1 VALUES(1, 'One', 101);
 UPDATE fdw126_ft1 SET stu_name = 'one' WHERE stu_id = 1;
 DELETE FROM fdw126_ft1 WHERE stu_id = 1;
 
@@ -90,6 +90,40 @@ ANALYZE f_empdata;
 ANALYZE f_empdata(emp_id);
 VACUUM ANALYZE f_empdata;
 
+-- Verify the before update trigger which modifies the column value which is not
+-- part of update statement.
+CREATE FUNCTION before_row_update_func() RETURNS TRIGGER AS $$
+BEGIN
+  NEW.stu_name := NEW.stu_name || ' trigger updated!';
+	RETURN NEW;
+  END
+$$ language plpgsql;
+
+CREATE TRIGGER before_row_update_trig
+BEFORE UPDATE ON fdw126_ft1
+FOR EACH ROW EXECUTE PROCEDURE before_row_update_func();
+
+INSERT INTO fdw126_ft1 VALUES(1, 'One', 101);
+EXPLAIN (verbose, costs off)
+UPDATE fdw126_ft1 SET stu_dept = 201 WHERE stu_id = 1;
+UPDATE fdw126_ft1 SET stu_dept = 201 WHERE stu_id = 1;
+SELECT * FROM fdw126_ft1 ORDER BY stu_id;
+
+-- Throw an error when target list has row identifier column.
+UPDATE fdw126_ft1 SET stu_dept = 201, stu_id = 10  WHERE stu_id = 1;
+
+-- Throw an error when before row update trigger modify the row identifier
+-- column value.
+CREATE OR REPLACE FUNCTION before_row_update_func() RETURNS TRIGGER AS $$
+BEGIN
+  NEW.stu_name := NEW.stu_name || ' trigger updated!';
+  NEW.stu_id = 20;
+	RETURN NEW;
+  END
+$$ language plpgsql;
+
+UPDATE fdw126_ft1 SET stu_dept = 301 WHERE stu_id = 1;
+
 -- Cleanup
 DELETE FROM fdw126_ft1;
 DELETE FROM f_empdata;
@@ -101,6 +135,7 @@ DROP FOREIGN TABLE fdw126_ft4;
 DROP FOREIGN TABLE fdw126_ft5;
 DROP FOREIGN TABLE fdw126_ft6;
 DROP FOREIGN TABLE f_empdata;
+DROP FUNCTION before_row_update_func();
 DROP USER MAPPING FOR public SERVER mysql_svr;
 DROP SERVER mysql_svr;
 DROP EXTENSION mysql_fdw;