Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> writes:
> The patch basically looks good to me except a minor comment to have a
> local variable for var->varattno which makes the code shorter.
Here's a restructured version that uses rangetable data for the
simple-relation case too. I also modified the relevant test cases
so that it's visible that we're reporting aliases not true names.
regards, tom lane
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 31b5de91ad..25112df916 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -4096,15 +4096,17 @@ DROP TABLE reind_fdw_parent;
-- conversion error
-- ===================================================================
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE int;
-SELECT * FROM ft1 WHERE c1 = 1; -- ERROR
+SELECT * FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8) WHERE x1 = 1; -- ERROR
ERROR: invalid input syntax for type integer: "foo"
-CONTEXT: column "c8" of foreign table "ft1"
-SELECT ft1.c1, ft2.c2, ft1.c8 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
+CONTEXT: column "x8" of foreign table "ftx"
+SELECT ftx.x1, ft2.c2, ftx.x8 FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
+ WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
ERROR: invalid input syntax for type integer: "foo"
-CONTEXT: column "c8" of foreign table "ft1"
-SELECT ft1.c1, ft2.c2, ft1 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
+CONTEXT: column "x8" of foreign table "ftx"
+SELECT ftx.x1, ft2.c2, ftx FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
+ WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
ERROR: invalid input syntax for type integer: "foo"
-CONTEXT: whole-row reference to foreign table "ft1"
+CONTEXT: whole-row reference to foreign table "ftx"
SELECT sum(c2), array_agg(c8) FROM ft1 GROUP BY c8; -- ERROR
ERROR: invalid input syntax for type integer: "foo"
CONTEXT: processing expression at position 2 in select list
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fafbab6b02..696277ba10 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -302,16 +302,8 @@ typedef struct
*/
typedef struct ConversionLocation
{
- Relation rel; /* foreign table's relcache entry. */
AttrNumber cur_attno; /* attribute number being processed, or 0 */
-
- /*
- * In case of foreign join push down, fdw_scan_tlist is used to identify
- * the Var node corresponding to the error location and
- * fsstate->ss.ps.state gives access to the RTEs of corresponding relation
- * to get the relation name and attribute name.
- */
- ForeignScanState *fsstate;
+ ForeignScanState *fsstate; /* plan node being processed */
} ConversionLocation;
/* Callback argument for ec_member_matches_foreign */
@@ -7082,7 +7074,6 @@ make_tuple_from_result_row(PGresult *res,
/*
* Set up and install callback to report where conversion error occurs.
*/
- errpos.rel = rel;
errpos.cur_attno = 0;
errpos.fsstate = fsstate;
errcallback.callback = conversion_error_callback;
@@ -7185,34 +7176,32 @@ make_tuple_from_result_row(PGresult *res,
/*
* Callback function which is called when error occurs during column value
* conversion. Print names of column and relation.
+ *
+ * Note that this function mustn't do any catalog lookups, since we are in
+ * an already-failed transaction. Fortunately, we can get the needed info
+ * from the query's rangetable instead.
*/
static void
conversion_error_callback(void *arg)
{
+ ConversionLocation *errpos = (ConversionLocation *) arg;
+ ForeignScanState *fsstate = errpos->fsstate;
+ ForeignScan *fsplan = castNode(ForeignScan, fsstate->ss.ps.plan);
+ int varno = 0;
+ AttrNumber colno = 0;
const char *attname = NULL;
const char *relname = NULL;
bool is_wholerow = false;
- ConversionLocation *errpos = (ConversionLocation *) arg;
- if (errpos->rel)
+ if (fsplan->scan.scanrelid > 0)
{
/* error occurred in a scan against a foreign table */
- TupleDesc tupdesc = RelationGetDescr(errpos->rel);
- Form_pg_attribute attr = TupleDescAttr(tupdesc, errpos->cur_attno - 1);
-
- if (errpos->cur_attno > 0 && errpos->cur_attno <= tupdesc->natts)
- attname = NameStr(attr->attname);
- else if (errpos->cur_attno == SelfItemPointerAttributeNumber)
- attname = "ctid";
-
- relname = RelationGetRelationName(errpos->rel);
+ varno = fsplan->scan.scanrelid;
+ colno = errpos->cur_attno;
}
else
{
/* error occurred in a scan against a foreign join */
- ForeignScanState *fsstate = errpos->fsstate;
- ForeignScan *fsplan = castNode(ForeignScan, fsstate->ss.ps.plan);
- EState *estate = fsstate->ss.ps.state;
TargetEntry *tle;
tle = list_nth_node(TargetEntry, fsplan->fdw_scan_tlist,
@@ -7220,35 +7209,42 @@ conversion_error_callback(void *arg)
/*
* Target list can have Vars and expressions. For Vars, we can get
- * its relation, however for expressions we can't. Thus for
+ * some information, however for expressions we can't. Thus for
* expressions, just show generic context message.
*/
if (IsA(tle->expr, Var))
{
- RangeTblEntry *rte;
Var *var = (Var *) tle->expr;
- rte = exec_rt_fetch(var->varno, estate);
-
- if (var->varattno == 0)
- is_wholerow = true;
- else
- attname = get_attname(rte->relid, var->varattno, false);
-
- relname = get_rel_name(rte->relid);
+ varno = var->varno;
+ colno = var->varattno;
}
- else
- errcontext("processing expression at position %d in select list",
- errpos->cur_attno);
}
- if (relname)
+ if (varno > 0)
{
- if (is_wholerow)
- errcontext("whole-row reference to foreign table \"%s\"", relname);
- else if (attname)
- errcontext("column \"%s\" of foreign table \"%s\"", attname, relname);
+ EState *estate = fsstate->ss.ps.state;
+ RangeTblEntry *rte = exec_rt_fetch(varno, estate);
+
+ relname = rte->eref->aliasname;
+
+ if (colno == 0)
+ is_wholerow = true;
+ else if (colno > 0 &&
+ colno <= list_length(rte->eref->colnames))
+ attname = strVal(list_nth(rte->eref->colnames,
+ colno - 1));
+ else if (colno == SelfItemPointerAttributeNumber)
+ attname = "ctid";
}
+
+ if (relname && is_wholerow)
+ errcontext("whole-row reference to foreign table \"%s\"", relname);
+ else if (relname && attname)
+ errcontext("column \"%s\" of foreign table \"%s\"", attname, relname);
+ else
+ errcontext("processing expression at position %d in select list",
+ errpos->cur_attno);
}
/*
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 286dd99573..95862e38ed 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1129,9 +1129,11 @@ DROP TABLE reind_fdw_parent;
-- conversion error
-- ===================================================================
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE int;
-SELECT * FROM ft1 WHERE c1 = 1; -- ERROR
-SELECT ft1.c1, ft2.c2, ft1.c8 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
-SELECT ft1.c1, ft2.c2, ft1 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
+SELECT * FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8) WHERE x1 = 1; -- ERROR
+SELECT ftx.x1, ft2.c2, ftx.x8 FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
+ WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
+SELECT ftx.x1, ft2.c2, ftx FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
+ WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
SELECT sum(c2), array_agg(c8) FROM ft1 GROUP BY c8; -- ERROR
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
