From 252ec8e2f9c2f00b0deddf4b17d558d7390bf3b2 Mon Sep 17 00:00:00 2001 From: francois payette Date: Tue, 22 Sep 2020 17:52:15 -0400 Subject: [PATCH] patch bug fix 194 --- expected/dml.out | 49 ++++++++++++++++++++++++++++++-- mysql_fdw.c | 74 +++++++++++++++++++++++++++++++++++++++++++++--- mysql_init.sh | 2 +- sql/dml.sql | 39 +++++++++++++++++++++++-- 4 files changed, 155 insertions(+), 9 deletions(-) diff --git a/expected/dml.out b/expected/dml.out index e00b580..e40a61d 100644 --- 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 index 864cbc7..bd6ab0d 100644 --- a/mysql_fdw.c +++ b/mysql_fdw.c @@ -26,6 +26,7 @@ #include #include +#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 index 2f1f27c..af4cce0 100755 --- 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 index ed822f3..49c44d4 100644 --- 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;