Finish COPY support

To complete the previous commit, I added the following: - fix COPY with AFTER INSERT triggers - add regression tests There is also some unrelated refactoring for better readability. This closes #309.
laurenz · Apr 19, 2019 · eef02eb · eef02eb
1 parent 3da5dea
commit eef02eb
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 54 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,4 +1,8 @@
-Version 2.1.1
+Version 2.2.0
+  Enhancements:
+  - Add support for COPY to foreign tables (from PostgreSQL v11 on).
+    This caused a crash before, as reported by "jkldv".
+
   Bugfixes:
   - Fix crash or bad results with pushed down join queries.
     The query target list can change during query planning, but oracle_fdw

diff --git a/expected/oracle_fdw.out b/expected/oracle_fdw.out
@@ -339,14 +339,24 @@ SELECT * FROM qtest ORDER BY id;
 (4 rows)
 
 DELETE FROM qtest WHERE id = 5;
+/*
+ * Test COPY
+ */
+BEGIN;
+COPY shorty FROM STDIN;
+ROLLBACK;
 /*
  * Test triggers on foreign tables.
  */
 -- trigger function
 CREATE FUNCTION shorttrig() RETURNS trigger LANGUAGE plpgsql AS
 $$BEGIN
-   RAISE WARNING 'trigger % % OLD row: id = %, c = %', TG_WHEN, TG_OP, OLD.id, OLD.c;
-   RAISE WARNING 'trigger % % NEW row: id = %, c = %', TG_WHEN, TG_OP, NEW.id, NEW.c;
+   IF TG_OP IN ('UPDATE', 'DELETE') THEN
+      RAISE WARNING 'trigger % % OLD row: id = %, c = %', TG_WHEN, TG_OP, OLD.id, OLD.c;
+   END IF;
+   IF TG_OP IN ('INSERT', 'UPDATE') THEN
+      RAISE WARNING 'trigger % % NEW row: id = %, c = %', TG_WHEN, TG_OP, NEW.id, NEW.c;
+   END IF;
    RETURN NEW;
 END;$$;
 -- test BEFORE trigger
@@ -364,6 +374,15 @@ UPDATE shorty SET id = id + 1 WHERE id = 4;
 WARNING:  trigger AFTER UPDATE OLD row: id = 4, c = short     
 WARNING:  trigger AFTER UPDATE NEW row: id = 5, c = short     
 ROLLBACK;
+-- test AFTER INSERT trigger with COPY
+DROP TRIGGER shorttrig ON shorty;
+CREATE TRIGGER shorttrig AFTER INSERT ON shorty FOR EACH ROW EXECUTE PROCEDURE shorttrig();
+BEGIN;
+COPY shorty FROM STDIN;
+WARNING:  trigger AFTER INSERT NEW row: id = 42, c = hammer    
+WARNING:  trigger AFTER INSERT NEW row: id = 753, c = rom       
+WARNING:  trigger AFTER INSERT NEW row: id = 0, c = <NULL>
+ROLLBACK;
 /*
  * Test ORDER BY pushdown.
  */

diff --git a/oracle_fdw.c b/oracle_fdw.c
@@ -375,6 +375,7 @@ static void convertTuple(struct OracleFdwState *fdw_state, Datum *values, bool *
 static void errorContextCallback(void *arg);
 static bool hasTrigger(Relation rel, CmdType cmdtype);
 static void buildInsertQuery(StringInfo sql, struct OracleFdwState *fdwState);
+static void buildUpdateQuery(StringInfo sql, struct OracleFdwState *fdwState, List *targetAttrs);
 static void appendReturningClause(StringInfo sql, struct OracleFdwState *fdwState);
 #ifdef IMPORT_API
 static char *fold_case(char *name, fold_t foldcase);
@@ -1606,7 +1607,6 @@ oraclePlanForeignModify(PlannerInfo *root, ModifyTable *plan, Index resultRelati
 	List *returningList = NIL;
 	struct OracleFdwState *fdwState;
 	int attnum, i;
-	ListCell *cell;
 	bool has_trigger = false, firstcol;
 	char paramName[10];
 	TupleDesc tupdesc;
@@ -1768,52 +1768,7 @@ oraclePlanForeignModify(PlannerInfo *root, ModifyTable *plan, Index resultRelati
 
 			break;
 		case CMD_UPDATE:
-			appendStringInfo(&sql, "UPDATE %s SET ", fdwState->oraTable->name);
-
-			firstcol = true;
-			i = 0;
-			foreach(cell, targetAttrs)
-			{
-				/* find the corresponding oraTable entry */
-				while (i < fdwState->oraTable->ncols && fdwState->oraTable->cols[i]->pgattnum < lfirst_int(cell))
-					++i;
-				if (i == fdwState->oraTable->ncols)
-					break;
-
-				/* ignore columns that don't occur in the foreign table */
-				if (fdwState->oraTable->cols[i]->pgtype == 0)
-					continue;
-
-				/* check that the data types can be converted */
-				checkDataType(
-					fdwState->oraTable->cols[i]->oratype,
-					fdwState->oraTable->cols[i]->scale,
-					fdwState->oraTable->cols[i]->pgtype,
-					fdwState->oraTable->pgname,
-					fdwState->oraTable->cols[i]->pgname
-				);
-
-				/* add a parameter description for the column */
-				snprintf(paramName, 9, ":p%d", lfirst_int(cell));
-				addParam(&fdwState->paramList, paramName, fdwState->oraTable->cols[i]->pgtype,
-					fdwState->oraTable->cols[i]->oratype, i);
-
-				/* add the parameter name to the query */
-				if (firstcol)
-					firstcol = false;
-				else
-					appendStringInfo(&sql, ", ");
-
-				appendStringInfo(&sql, "%s = ", fdwState->oraTable->cols[i]->name);
-				appendAsType(&sql, paramName, fdwState->oraTable->cols[i]->pgtype);
-			}
-
-			/* throw a meaningful error if nothing is updated */
-			if (firstcol)
-				ereport(ERROR,
-						(errcode(ERRCODE_FDW_UNABLE_TO_CREATE_EXECUTION),
-						errmsg("no Oracle column modified by UPDATE"),
-						errdetail("The UPDATE statement only changes colums that do not exist in the Oracle table.")));
+			buildUpdateQuery(&sql, fdwState, targetAttrs);
 
 			break;
 		case CMD_DELETE:
@@ -1947,6 +1902,16 @@ void oracleBeginForeignInsert(ModifyTableState *mtstate, ResultRelInfo *rinfo)
 
 	fdw_state = getFdwState(RelationGetRelid(rinfo->ri_RelationDesc), NULL);
 
+	/* not using "deserializePlanData", we have to initialize these ourselves */
+	for (i=0; i<fdw_state->oraTable->ncols; ++i)
+	{
+		fdw_state->oraTable->cols[i]->val = (char *)palloc(fdw_state->oraTable->cols[i]->val_size + 1);
+		fdw_state->oraTable->cols[i]->val_len = 0;
+		fdw_state->oraTable->cols[i]->val_len4 = 0;
+		fdw_state->oraTable->cols[i]->val_null = 1;
+	}
+	fdw_state->rowcount = 0;
+
 	fdw_state->session = oracleGetSession(
 			fdw_state->dbserver,
 			fdw_state->user,
@@ -5171,6 +5136,7 @@ Const
 /*
  * deserializePlanData
  * 		Extract the data structures from a List created by serializePlanData.
+ * 		Allocates memory for values returned from Oracle.
  */
 
 struct OracleFdwState
@@ -5864,6 +5830,62 @@ buildInsertQuery(StringInfo sql, struct OracleFdwState *fdwState)
 	appendStringInfo(sql, ")");
 }
 
+void
+buildUpdateQuery(StringInfo sql, struct OracleFdwState *fdwState, List *targetAttrs)
+{
+	bool firstcol;
+	int i;
+	char paramName[10];
+	ListCell *cell;
+
+	appendStringInfo(sql, "UPDATE %s SET ", fdwState->oraTable->name);
+
+	firstcol = true;
+	i = 0;
+	foreach(cell, targetAttrs)
+	{
+		/* find the corresponding oraTable entry */
+		while (i < fdwState->oraTable->ncols && fdwState->oraTable->cols[i]->pgattnum < lfirst_int(cell))
+			++i;
+		if (i == fdwState->oraTable->ncols)
+			break;
+
+		/* ignore columns that don't occur in the foreign table */
+		if (fdwState->oraTable->cols[i]->pgtype == 0)
+			continue;
+
+		/* check that the data types can be converted */
+		checkDataType(
+			fdwState->oraTable->cols[i]->oratype,
+			fdwState->oraTable->cols[i]->scale,
+			fdwState->oraTable->cols[i]->pgtype,
+			fdwState->oraTable->pgname,
+			fdwState->oraTable->cols[i]->pgname
+		);
+
+		/* add a parameter description for the column */
+		snprintf(paramName, 9, ":p%d", lfirst_int(cell));
+		addParam(&fdwState->paramList, paramName, fdwState->oraTable->cols[i]->pgtype,
+			fdwState->oraTable->cols[i]->oratype, i);
+
+		/* add the parameter name to the query */
+		if (firstcol)
+			firstcol = false;
+		else
+			appendStringInfo(sql, ", ");
+
+		appendStringInfo(sql, "%s = ", fdwState->oraTable->cols[i]->name);
+		appendAsType(sql, paramName, fdwState->oraTable->cols[i]->pgtype);
+	}
+
+	/* throw a meaningful error if nothing is updated */
+	if (firstcol)
+		ereport(ERROR,
+				(errcode(ERRCODE_FDW_UNABLE_TO_CREATE_EXECUTION),
+				errmsg("no Oracle column modified by UPDATE"),
+				errdetail("The UPDATE statement only changes colums that do not exist in the Oracle table.")));
+}
+
 void
 appendReturningClause(StringInfo sql, struct OracleFdwState *fdwState)
 {
@@ -5907,7 +5929,7 @@ appendReturningClause(StringInfo sql, struct OracleFdwState *fdwState)
 			param->name = pstrdup(paramName);
 			param->type = fdwState->oraTable->cols[i]->pgtype;
 			param->bindType = BIND_OUTPUT;
-			param->value = NULL;
+			param->value = (void *)42;  /* something != NULL */
 			param->node = NULL;
 			param->bindh = NULL;
 			param->colnum = i;

diff --git a/oracle_fdw.h b/oracle_fdw.h
@@ -15,7 +15,7 @@
 #include <sys/types.h>
 
 /* oracle_fdw version */
-#define ORACLE_FDW_VERSION "2.1.1devel"
+#define ORACLE_FDW_VERSION "2.2.0devel"
 
 #ifdef OCI_ORACLE
 /*

diff --git a/sql/oracle_fdw.sql b/sql/oracle_fdw.sql
@@ -234,15 +234,31 @@ ALTER FOREIGN TABLE qtest OPTIONS (SET table '(SELECT id, SUBSTR(vc, 1, 3), num
 SELECT * FROM qtest ORDER BY id;
 DELETE FROM qtest WHERE id = 5;
 
+/*
+ * Test COPY
+ */
+
+BEGIN;
+COPY shorty FROM STDIN;
+666	devil
+777	lucky
+0	\N
+\.
+ROLLBACK;
+
 /*
  * Test triggers on foreign tables.
  */
 
 -- trigger function
 CREATE FUNCTION shorttrig() RETURNS trigger LANGUAGE plpgsql AS
 $$BEGIN
-   RAISE WARNING 'trigger % % OLD row: id = %, c = %', TG_WHEN, TG_OP, OLD.id, OLD.c;
-   RAISE WARNING 'trigger % % NEW row: id = %, c = %', TG_WHEN, TG_OP, NEW.id, NEW.c;
+   IF TG_OP IN ('UPDATE', 'DELETE') THEN
+      RAISE WARNING 'trigger % % OLD row: id = %, c = %', TG_WHEN, TG_OP, OLD.id, OLD.c;
+   END IF;
+   IF TG_OP IN ('INSERT', 'UPDATE') THEN
+      RAISE WARNING 'trigger % % NEW row: id = %, c = %', TG_WHEN, TG_OP, NEW.id, NEW.c;
+   END IF;
    RETURN NEW;
 END;$$;
 
@@ -259,6 +275,17 @@ BEGIN;
 UPDATE shorty SET id = id + 1 WHERE id = 4;
 ROLLBACK;
 
+-- test AFTER INSERT trigger with COPY
+DROP TRIGGER shorttrig ON shorty;
+CREATE TRIGGER shorttrig AFTER INSERT ON shorty FOR EACH ROW EXECUTE PROCEDURE shorttrig();
+BEGIN;
+COPY shorty FROM STDIN;
+42	hammer
+753	rom
+0	\N
+\.
+ROLLBACK;
+
 /*
  * Test ORDER BY pushdown.
  */