aboutsummaryrefslogtreecommitdiff
path: root/contrib/postgres_fdw/postgres_fdw.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2021-07-06 12:36:12 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2021-07-06 12:36:12 -0400
commitc7b7311f6177b90fe5033b8ed2c10dfcbb863eb6 (patch)
treea6bf224661f36a6ef413c00b0eadac37729e68cd /contrib/postgres_fdw/postgres_fdw.c
parentab2e19987ff66f83dfb99b5c541d9e05e8b0ede3 (diff)
downloadpostgresql-c7b7311f6177b90fe5033b8ed2c10dfcbb863eb6.tar.gz
postgresql-c7b7311f6177b90fe5033b8ed2c10dfcbb863eb6.zip
Avoid doing catalog lookups in postgres_fdw's conversion_error_callback.
As in 50371df26, this is a bad idea since the callback can't really know what error is being thrown and thus whether or not it is safe to attempt catalog accesses. Rather than pushing said accesses into the mainline code where they'd usually be a waste of cycles, we can look at the query's rangetable instead. This change does mean that we'll be printing query aliases (if any were used) rather than the table or column's true name. But that doesn't seem like a bad thing: it's certainly a more useful definition in self-join cases, for instance. In any case, it seems unlikely that any applications would be depending on this detail, so it seems safe to change. Patch by me. Original complaint by Andres Freund; Bharath Rupireddy noted the connection to conversion_error_callback. Discussion: https://postgr.es/m/20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
Diffstat (limited to 'contrib/postgres_fdw/postgres_fdw.c')
-rw-r--r--contrib/postgres_fdw/postgres_fdw.c78
1 files changed, 36 insertions, 42 deletions
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fafbab6b024..a5afac47ebd 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,40 @@ 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);
}
/*