Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commita9460db

Browse files
committed
Avoid doing catalog lookups in postgres_fdw's conversion_error_callback.
As in50371df, this is a bad idea since the callback can't reallyknow what error is being thrown and thus whether or not it is safeto attempt catalog accesses. Rather than pushing said accesses intothe mainline code where they'd usually be a waste of cycles, we canlook at the query's rangetable instead.This change does mean that we'll be printing query aliases (if anywere used) rather than the table or column's true name. But thatdoesn't seem like a bad thing: it's certainly a more useful definitionin self-join cases, for instance. In any case, it seems unlikely thatany applications would be depending on this detail, so it seems safeto change.Patch by me. Original complaint by Andres Freund; Bharath Rupireddynoted the connection to conversion_error_callback.Discussion:https://postgr.es/m/20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
1 parent151d30e commita9460db

File tree

3 files changed

+51
-53
lines changed

3 files changed

+51
-53
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4190,15 +4190,17 @@ DROP FUNCTION f_test(int);
41904190
-- conversion error
41914191
-- ===================================================================
41924192
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE int;
4193-
SELECT * FROM ft1 WHEREc1 = 1; -- ERROR
4193+
SELECT * FROM ft1ftx(x1,x2,x3,x4,x5,x6,x7,x8)WHEREx1 = 1; -- ERROR
41944194
ERROR: invalid input syntax for integer: "foo"
4195-
CONTEXT: column "c8" of foreign table "ft1"
4196-
SELECT ft1.c1, ft2.c2, ft1.c8 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
4195+
CONTEXT: column "x8" of foreign table "ftx"
4196+
SELECT ftx.x1, ft2.c2, ftx.x8 FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
4197+
WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
41974198
ERROR: invalid input syntax for integer: "foo"
4198-
CONTEXT: column "c8" of foreign table "ft1"
4199-
SELECT ft1.c1, ft2.c2, ft1 FROM ft1, ft2 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = 1; -- ERROR
4199+
CONTEXT: column "x8" of foreign table "ftx"
4200+
SELECT ftx.x1, ft2.c2, ftx FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
4201+
WHERE ftx.x1 = ft2.c1 AND ftx.x1 = 1; -- ERROR
42004202
ERROR: invalid input syntax for integer: "foo"
4201-
CONTEXT: whole-row reference to foreign table "ft1"
4203+
CONTEXT: whole-row reference to foreign table "ftx"
42024204
SELECT sum(c2), array_agg(c8) FROM ft1 GROUP BY c8; -- ERROR
42034205
ERROR: invalid input syntax for integer: "foo"
42044206
CONTEXT: processing expression at position 2 in select list

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 38 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,8 @@ typedef struct PgFdwAnalyzeState
253253
*/
254254
typedefstructConversionLocation
255255
{
256-
Relationrel;/* foreign table's relcache entry. */
257256
AttrNumbercur_attno;/* attribute number being processed, or 0 */
258-
259-
/*
260-
* In case of foreign join push down, fdw_scan_tlist is used to identify
261-
* the Var node corresponding to the error location and
262-
* fsstate->ss.ps.state gives access to the RTEs of corresponding relation
263-
* to get the relation name and attribute name.
264-
*/
265-
ForeignScanState*fsstate;
257+
ForeignScanState*fsstate;/* plan node being processed */
266258
}ConversionLocation;
267259

268260
/* Callback argument for ec_member_matches_foreign */
@@ -5695,7 +5687,6 @@ make_tuple_from_result_row(PGresult *res,
56955687
/*
56965688
* Set up and install callback to report where conversion error occurs.
56975689
*/
5698-
errpos.rel=rel;
56995690
errpos.cur_attno=0;
57005691
errpos.fsstate=fsstate;
57015692
errcallback.callback=conversion_error_callback;
@@ -5815,72 +5806,75 @@ make_tuple_from_result_row(PGresult *res,
58155806
/*
58165807
* Callback function which is called when error occurs during column value
58175808
* conversion. Print names of column and relation.
5809+
*
5810+
* Note that this function mustn't do any catalog lookups, since we are in
5811+
* an already-failed transaction. Fortunately, we can get the needed info
5812+
* from the query's rangetable instead.
58185813
*/
58195814
staticvoid
58205815
conversion_error_callback(void*arg)
58215816
{
5817+
ConversionLocation*errpos= (ConversionLocation*)arg;
5818+
ForeignScanState*fsstate=errpos->fsstate;
5819+
ForeignScan*fsplan=castNode(ForeignScan,fsstate->ss.ps.plan);
5820+
intvarno=0;
5821+
AttrNumbercolno=0;
58225822
constchar*attname=NULL;
58235823
constchar*relname=NULL;
58245824
boolis_wholerow= false;
5825-
ConversionLocation*errpos= (ConversionLocation*)arg;
58265825

5827-
if (errpos->rel)
5826+
if (fsplan->scan.scanrelid>0)
58285827
{
58295828
/* error occurred in a scan against a foreign table */
5830-
TupleDesctupdesc=RelationGetDescr(errpos->rel);
5831-
Form_pg_attributeattr=TupleDescAttr(tupdesc,errpos->cur_attno-1);
5832-
5833-
if (errpos->cur_attno>0&&errpos->cur_attno <=tupdesc->natts)
5834-
attname=NameStr(attr->attname);
5835-
elseif (errpos->cur_attno==SelfItemPointerAttributeNumber)
5836-
attname="ctid";
5837-
elseif (errpos->cur_attno==ObjectIdAttributeNumber)
5838-
attname="oid";
5839-
5840-
relname=RelationGetRelationName(errpos->rel);
5829+
varno=fsplan->scan.scanrelid;
5830+
colno=errpos->cur_attno;
58415831
}
58425832
else
58435833
{
58445834
/* error occurred in a scan against a foreign join */
5845-
ForeignScanState*fsstate=errpos->fsstate;
5846-
ForeignScan*fsplan=castNode(ForeignScan,fsstate->ss.ps.plan);
5847-
EState*estate=fsstate->ss.ps.state;
58485835
TargetEntry*tle;
58495836

58505837
tle=list_nth_node(TargetEntry,fsplan->fdw_scan_tlist,
58515838
errpos->cur_attno-1);
58525839

58535840
/*
58545841
* Target list can have Vars and expressions. For Vars, we can get
5855-
*its relation, however for expressions we can't. Thus for
5842+
*some information, however for expressions we can't. Thus for
58565843
* expressions, just show generic context message.
58575844
*/
58585845
if (IsA(tle->expr,Var))
58595846
{
5860-
RangeTblEntry*rte;
58615847
Var*var= (Var*)tle->expr;
58625848

5863-
rte=rt_fetch(var->varno,estate->es_range_table);
5864-
5865-
if (var->varattno==0)
5866-
is_wholerow= true;
5867-
else
5868-
attname=get_attname(rte->relid,var->varattno, false);
5869-
5870-
relname=get_rel_name(rte->relid);
5849+
varno=var->varno;
5850+
colno=var->varattno;
58715851
}
5872-
else
5873-
errcontext("processing expression at position %d in select list",
5874-
errpos->cur_attno);
58755852
}
58765853

5877-
if (relname)
5854+
if (varno>0)
58785855
{
5879-
if (is_wholerow)
5880-
errcontext("whole-row reference to foreign table \"%s\"",relname);
5881-
elseif (attname)
5882-
errcontext("column \"%s\" of foreign table \"%s\"",attname,relname);
5856+
EState*estate=fsstate->ss.ps.state;
5857+
RangeTblEntry*rte=rt_fetch(varno,estate->es_range_table);
5858+
5859+
relname=rte->eref->aliasname;
5860+
5861+
if (colno==0)
5862+
is_wholerow= true;
5863+
elseif (colno>0&&colno <=list_length(rte->eref->colnames))
5864+
attname=strVal(list_nth(rte->eref->colnames,colno-1));
5865+
elseif (colno==SelfItemPointerAttributeNumber)
5866+
attname="ctid";
5867+
elseif (colno==ObjectIdAttributeNumber)
5868+
attname="oid";
58835869
}
5870+
5871+
if (relname&&is_wholerow)
5872+
errcontext("whole-row reference to foreign table \"%s\"",relname);
5873+
elseif (relname&&attname)
5874+
errcontext("column \"%s\" of foreign table \"%s\"",attname,relname);
5875+
else
5876+
errcontext("processing expression at position %d in select list",
5877+
errpos->cur_attno);
58845878
}
58855879

58865880
/*

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,9 +1061,11 @@ DROP FUNCTION f_test(int);
10611061
-- conversion error
10621062
-- ===================================================================
10631063
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPEint;
1064-
SELECT*FROM ft1WHERE c1=1;-- ERROR
1065-
SELECTft1.c1,ft2.c2,ft1.c8FROM ft1, ft2WHEREft1.c1=ft2.c1ANDft1.c1=1;-- ERROR
1066-
SELECTft1.c1,ft2.c2, ft1FROM ft1, ft2WHEREft1.c1=ft2.c1ANDft1.c1=1;-- ERROR
1064+
SELECT*FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8)WHERE x1=1;-- ERROR
1065+
SELECTftx.x1,ft2.c2,ftx.x8FROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
1066+
WHEREftx.x1=ft2.c1ANDftx.x1=1;-- ERROR
1067+
SELECTftx.x1,ft2.c2, ftxFROM ft1 ftx(x1,x2,x3,x4,x5,x6,x7,x8), ft2
1068+
WHEREftx.x1=ft2.c1ANDftx.x1=1;-- ERROR
10671069
SELECTsum(c2), array_agg(c8)FROM ft1GROUP BY c8;-- ERROR
10681070
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
10691071

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp