From 79b37b9160572a9d730c97d9fc1f471064b26c7e Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Mon, 15 Jun 2020 10:43:05 +0530 Subject: [PATCH 2/2] Separate code to list all columns of a foreign relation to in its own function This code resides in deparseAnalyzeSql() but is useful for COPY as well. So separate it into its own function. This takes care of any dropped columns or no columns cases. However COPY command constructed for a relation which doesn't have any non-dropped columns locally but has some columns on the remote server has a syntax error. This needs to be fixed. Ashutosh Bapat --- contrib/postgres_fdw/deparse.c | 58 +++++++++++-------- .../postgres_fdw/expected/postgres_fdw.out | 16 ++++- contrib/postgres_fdw/sql/postgres_fdw.sql | 19 ++++++ 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 427402c8eb..aa06342bfa 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -184,6 +184,8 @@ static void appendAggOrderBy(List *orderList, List *targetList, static void appendFunctionName(Oid funcid, deparse_expr_cxt *context); static Node *deparseSortGroupClause(Index ref, List *tlist, bool force_colno, deparse_expr_cxt *context); +static List *deparseRelColumnList(StringInfo buf, Relation rel, + bool enclose_in_parens); /* * Helper functions @@ -1769,17 +1771,7 @@ deparseCopyFromSql(StringInfo buf, Relation rel) appendStringInfoString(buf, "COPY "); deparseRelation(buf, rel); - appendStringInfoString(buf, " ( "); - - for(attnum = 0; attnum < rel->rd_att->natts; attnum++) - { - appendStringInfoString(buf, NameStr(rel->rd_att->attrs[attnum].attname)); - - if (attnum != rel->rd_att->natts-1) - appendStringInfoString(buf, ", "); - } - - appendStringInfoString(buf, " ) "); + (void) deparseRelColumnList(buf, rel, true); appendStringInfoString(buf, " FROM STDIN "); } @@ -2086,6 +2078,30 @@ deparseAnalyzeSizeSql(StringInfo buf, Relation rel) */ void deparseAnalyzeSql(StringInfo buf, Relation rel, List **retrieved_attrs) +{ + appendStringInfoString(buf, "SELECT "); + *retrieved_attrs = deparseRelColumnList(buf, rel, false); + + /* Don't generate bad syntax for zero-column relation. */ + if (list_length(*retrieved_attrs) == 0) + appendStringInfoString(buf, "NULL"); + + /* + * Construct FROM clause + */ + appendStringInfoString(buf, " FROM "); + deparseRelation(buf, rel); +} + +/* + * Construct the list of columns of given foreign relation in the order they + * appear in the tuple descriptor of the relation. Ignore any dropped columns. + * Use column names on the foreign server instead of local names. + * + * Optionally enclose the list in parantheses. + */ +static List * +deparseRelColumnList(StringInfo buf, Relation rel, bool enclose_in_parens) { Oid relid = RelationGetRelid(rel); TupleDesc tupdesc = RelationGetDescr(rel); @@ -2094,10 +2110,8 @@ deparseAnalyzeSql(StringInfo buf, Relation rel, List **retrieved_attrs) List *options; ListCell *lc; bool first = true; + List *retrieved_attrs = NIL; - *retrieved_attrs = NIL; - - appendStringInfoString(buf, "SELECT "); for (i = 0; i < tupdesc->natts; i++) { /* Ignore dropped columns. */ @@ -2106,6 +2120,9 @@ deparseAnalyzeSql(StringInfo buf, Relation rel, List **retrieved_attrs) if (!first) appendStringInfoString(buf, ", "); + else if (enclose_in_parens) + appendStringInfoChar(buf, '('); + first = false; /* Use attribute name or column_name option. */ @@ -2125,18 +2142,13 @@ deparseAnalyzeSql(StringInfo buf, Relation rel, List **retrieved_attrs) appendStringInfoString(buf, quote_identifier(colname)); - *retrieved_attrs = lappend_int(*retrieved_attrs, i + 1); + retrieved_attrs = lappend_int(retrieved_attrs, i + 1); } - /* Don't generate bad syntax for zero-column relation. */ - if (first) - appendStringInfoString(buf, "NULL"); + if (enclose_in_parens && list_length(retrieved_attrs) > 0) + appendStringInfoChar(buf, ')'); - /* - * Construct FROM clause - */ - appendStringInfoString(buf, " FROM "); - deparseRelation(buf, rel); + return retrieved_attrs; } /* diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 922c08d2dc..e4201a7779 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -8064,7 +8064,7 @@ copy rem2 from stdin; -- ERROR ERROR: new row for relation "loc2" violates check constraint "loc2_f1positive" DETAIL: Failing row contains (-1, xyzzy). CONTEXT: COPY loc2, line 1: "-1 xyzzy" -remote SQL command: COPY public.loc2 ( f1, f2 ) FROM STDIN +remote SQL command: COPY public.loc2(f1, f2) FROM STDIN COPY rem2, line 2 select * from rem2; f1 | f2 @@ -8184,6 +8184,20 @@ drop trigger rem2_trig_row_before on rem2; drop trigger rem2_trig_row_after on rem2; drop trigger loc2_trig_row_before_insert on loc2; delete from rem2; +-- dropped columns locally and on the foreign server +alter table rem2 drop column f1; +alter table rem2 drop column f2; +-- this will error because of data and column mismatch +-- FIXME +copy rem2 from stdin; +select * from rem2; +alter table loc2 drop column f1; +alter table loc2 drop column f2; +copy rem2 from stdin; +select * from rem2; +-- +(2 rows) + -- test COPY FROM with foreign table created in the same transaction create table loc3 (f1 int, f2 text); begin; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 83971665e3..37f0c61183 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2293,6 +2293,25 @@ drop trigger loc2_trig_row_before_insert on loc2; delete from rem2; +-- dropped columns locally and on the foreign server +alter table rem2 drop column f1; +alter table rem2 drop column f2; +-- this will error because of data and column mismatch +-- FIXME +copy rem2 from stdin; + + +\. +select * from rem2; + +alter table loc2 drop column f1; +alter table loc2 drop column f2; +copy rem2 from stdin; + + +\. +select * from rem2; + -- test COPY FROM with foreign table created in the same transaction create table loc3 (f1 int, f2 text); begin; -- 2.17.1