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

Commita620d50

Browse files
committed
Fix a bug introduced when set-returning SQL functions were made inline-able:
we have to cope with the possibility that the declared result rowtype containsdropped columns. This fails in 8.4, as per bug #5240.While at it, be more paranoid about inserting binary coercions when inlining.The pre-8.4 code did not really need to worry about that because it could notinline at all in any case where an added coercion could change the behaviorof the function's statement. However, when inlining a SRF we allow sorting,grouping, and set-ops such as UNION. In these cases, modifying one of thetargetlist entries that the sort/group/setop depends on could conceivablychange the behavior of the function's statement --- so don't inline whensuch a case applies.
1 parent84f910a commita620d50

File tree

7 files changed

+247
-40
lines changed

7 files changed

+247
-40
lines changed

‎src/backend/catalog/pg_proc.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.168 2009/10/08 02:39:18 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.169 2009/12/14 02:15:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -810,7 +810,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
810810
proc->pronargs);
811811
(void)check_sql_fn_retval(funcoid,proc->prorettype,
812812
querytree_list,
813-
false,NULL);
813+
NULL,NULL);
814814
}
815815
else
816816
querytree_list=pg_parse_query(prosrc);

‎src/backend/executor/execQual.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.255 2009/11/20 20:38:10 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.256 2009/12/14 02:15:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -47,6 +47,7 @@
4747
#include"nodes/makefuncs.h"
4848
#include"nodes/nodeFuncs.h"
4949
#include"optimizer/planner.h"
50+
#include"parser/parse_coerce.h"
5051
#include"pgstat.h"
5152
#include"utils/acl.h"
5253
#include"utils/builtins.h"
@@ -1382,7 +1383,7 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
13821383
Form_pg_attributedattr=dst_tupdesc->attrs[i];
13831384
Form_pg_attributesattr=src_tupdesc->attrs[i];
13841385

1385-
if (dattr->atttypid==sattr->atttypid)
1386+
if (IsBinaryCoercible(sattr->atttypid,dattr->atttypid))
13861387
continue;/* no worries */
13871388
if (!dattr->attisdropped)
13881389
ereport(ERROR,

‎src/backend/executor/functions.c‎

Lines changed: 124 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.136 2009/11/04 22:26:05 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.137 2009/12/14 02:15:51 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -338,7 +338,7 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK)
338338
fcache->returnsTuple=check_sql_fn_retval(foid,
339339
rettype,
340340
queryTree_list,
341-
false,
341+
NULL,
342342
&fcache->junkFilter);
343343

344344
if (fcache->returnsTuple)
@@ -1003,14 +1003,27 @@ ShutdownSQLFunction(Datum arg)
10031003
* function definition of a polymorphic function.)
10041004
*
10051005
* This function returns true if the sql function returns the entire tuple
1006-
* result of its final statement,and falseotherwise.Note that because we
1007-
*allow "SELECT rowtype_expression", this may be false even whenthedeclared
1008-
*functionreturntype is a rowtype.
1006+
* result of its final statement,or falseif it returns just the first column
1007+
*result of that statement. It throws an error ifthefinal statement doesn't
1008+
* returnthe right type at all.
10091009
*
1010-
* If insertRelabels is true, then binary-compatible cases are dealt with
1011-
* by actually inserting RelabelType nodes into the output targetlist;
1012-
* obviously the caller must pass a parsetree that it's okay to modify in this
1013-
* case.
1010+
* Note that because we allow "SELECT rowtype_expression", the result can be
1011+
* false even when the declared function return type is a rowtype.
1012+
*
1013+
* If modifyTargetList isn't NULL, the function will modify the final
1014+
* statement's targetlist in two cases:
1015+
* (1) if the tlist returns values that are binary-coercible to the expected
1016+
* type rather than being exactly the expected type. RelabelType nodes will
1017+
* be inserted to make the result types match exactly.
1018+
* (2) if there are dropped columns in the declared result rowtype. NULL
1019+
* output columns will be inserted in the tlist to match them.
1020+
* (Obviously the caller must pass a parsetree that is okay to modify when
1021+
* using this flag.) Note that this flag does not affect whether the tlist is
1022+
* considered to be a legal match to the result type, only how we react to
1023+
* allowed not-exact-match cases. *modifyTargetList will be set true iff
1024+
* we had to make any "dangerous" changes that could modify the semantics of
1025+
* the statement. If it is set true, the caller should not use the modified
1026+
* statement, but for simplicity we apply the changes anyway.
10141027
*
10151028
* If junkFilter isn't NULL, then *junkFilter is set to a JunkFilter defined
10161029
* to convert the function's tuple result to the correct output tuple type.
@@ -1019,10 +1032,11 @@ ShutdownSQLFunction(Datum arg)
10191032
*/
10201033
bool
10211034
check_sql_fn_retval(Oidfunc_id,Oidrettype,List*queryTreeList,
1022-
boolinsertRelabels,
1035+
bool*modifyTargetList,
10231036
JunkFilter**junkFilter)
10241037
{
10251038
Query*parse;
1039+
List**tlist_ptr;
10261040
List*tlist;
10271041
inttlistlen;
10281042
charfn_typtype;
@@ -1031,6 +1045,8 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
10311045

10321046
AssertArg(!IsPolymorphicType(rettype));
10331047

1048+
if (modifyTargetList)
1049+
*modifyTargetList= false;/* initialize for no change */
10341050
if (junkFilter)
10351051
*junkFilter=NULL;/* initialize in case of VOID result */
10361052

@@ -1064,6 +1080,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
10641080
parse->utilityStmt==NULL&&
10651081
parse->intoClause==NULL)
10661082
{
1083+
tlist_ptr=&parse->targetList;
10671084
tlist=parse->targetList;
10681085
}
10691086
elseif (parse&&
@@ -1072,6 +1089,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
10721089
parse->commandType==CMD_DELETE)&&
10731090
parse->returningList)
10741091
{
1092+
tlist_ptr=&parse->returningList;
10751093
tlist=parse->returningList;
10761094
}
10771095
else
@@ -1132,11 +1150,16 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
11321150
format_type_be(rettype)),
11331151
errdetail("Actual return type is %s.",
11341152
format_type_be(restype))));
1135-
if (insertRelabels&&restype!=rettype)
1153+
if (modifyTargetList&&restype!=rettype)
1154+
{
11361155
tle->expr= (Expr*)makeRelabelType(tle->expr,
11371156
rettype,
11381157
-1,
11391158
COERCE_DONTCARE);
1159+
/* Relabel is dangerous if TLE is a sort/group or setop column */
1160+
if (tle->ressortgroupref!=0||parse->setOperations)
1161+
*modifyTargetList= true;
1162+
}
11401163

11411164
/* Set up junk filter if needed */
11421165
if (junkFilter)
@@ -1149,6 +1172,8 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
11491172
inttupnatts;/* physical number of columns in tuple */
11501173
inttuplogcols;/* # of nondeleted columns in tuple */
11511174
intcolindex;/* physical column index */
1175+
List*newtlist;/* new non-junk tlist entries */
1176+
List*junkattrs;/* new junk tlist entries */
11521177

11531178
/*
11541179
* If the target list is of length 1, and the type of the varnode in
@@ -1165,11 +1190,16 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
11651190
restype=exprType((Node*)tle->expr);
11661191
if (IsBinaryCoercible(restype,rettype))
11671192
{
1168-
if (insertRelabels&&restype!=rettype)
1193+
if (modifyTargetList&&restype!=rettype)
1194+
{
11691195
tle->expr= (Expr*)makeRelabelType(tle->expr,
11701196
rettype,
11711197
-1,
11721198
COERCE_DONTCARE);
1199+
/* Relabel is dangerous if sort/group or setop column */
1200+
if (tle->ressortgroupref!=0||parse->setOperations)
1201+
*modifyTargetList= true;
1202+
}
11731203
/* Set up junk filter if needed */
11741204
if (junkFilter)
11751205
*junkFilter=ExecInitJunkFilter(tlist, false,NULL);
@@ -1193,11 +1223,14 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
11931223
/*
11941224
* Verify that the targetlist matches the return tuple type. We scan
11951225
* the non-deleted attributes to ensure that they match the datatypes
1196-
* of the non-resjunk columns.
1226+
* of the non-resjunk columns. For deleted attributes, insert NULL
1227+
* result columns if the caller asked for that.
11971228
*/
11981229
tupnatts=tupdesc->natts;
11991230
tuplogcols=0;/* we'll count nondeleted cols as we go */
12001231
colindex=0;
1232+
newtlist=NIL;/* these are only used if modifyTargetList */
1233+
junkattrs=NIL;
12011234

12021235
foreach(lc,tlist)
12031236
{
@@ -1207,7 +1240,11 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
12071240
Oidatttype;
12081241

12091242
if (tle->resjunk)
1243+
{
1244+
if (modifyTargetList)
1245+
junkattrs=lappend(junkattrs,tle);
12101246
continue;
1247+
}
12111248

12121249
do
12131250
{
@@ -1219,6 +1256,26 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
12191256
format_type_be(rettype)),
12201257
errdetail("Final statement returns too many columns.")));
12211258
attr=tupdesc->attrs[colindex-1];
1259+
if (attr->attisdropped&&modifyTargetList)
1260+
{
1261+
Expr*null_expr;
1262+
1263+
/* The type of the null we insert isn't important */
1264+
null_expr= (Expr*)makeConst(INT4OID,
1265+
-1,
1266+
sizeof(int32),
1267+
(Datum)0,
1268+
true,/* isnull */
1269+
true/* byval */ );
1270+
newtlist=lappend(newtlist,
1271+
makeTargetEntry(null_expr,
1272+
colindex,
1273+
NULL,
1274+
false));
1275+
/* NULL insertion is dangerous in a setop */
1276+
if (parse->setOperations)
1277+
*modifyTargetList= true;
1278+
}
12221279
}while (attr->attisdropped);
12231280
tuplogcols++;
12241281

@@ -1233,28 +1290,66 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
12331290
format_type_be(tletype),
12341291
format_type_be(atttype),
12351292
tuplogcols)));
1236-
if (insertRelabels&&tletype!=atttype)
1237-
tle->expr= (Expr*)makeRelabelType(tle->expr,
1238-
atttype,
1239-
-1,
1240-
COERCE_DONTCARE);
1293+
if (modifyTargetList)
1294+
{
1295+
if (tletype!=atttype)
1296+
{
1297+
tle->expr= (Expr*)makeRelabelType(tle->expr,
1298+
atttype,
1299+
-1,
1300+
COERCE_DONTCARE);
1301+
/* Relabel is dangerous if sort/group or setop column */
1302+
if (tle->ressortgroupref!=0||parse->setOperations)
1303+
*modifyTargetList= true;
1304+
}
1305+
tle->resno=colindex;
1306+
newtlist=lappend(newtlist,tle);
1307+
}
12411308
}
12421309

1243-
for (;;)
1310+
/* remaining columns in tupdesc had better all be dropped */
1311+
for (colindex++;colindex <=tupnatts;colindex++)
12441312
{
1245-
colindex++;
1246-
if (colindex>tupnatts)
1247-
break;
12481313
if (!tupdesc->attrs[colindex-1]->attisdropped)
1249-
tuplogcols++;
1314+
ereport(ERROR,
1315+
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
1316+
errmsg("return type mismatch in function declared to return %s",
1317+
format_type_be(rettype)),
1318+
errdetail("Final statement returns too few columns.")));
1319+
if (modifyTargetList)
1320+
{
1321+
Expr*null_expr;
1322+
1323+
/* The type of the null we insert isn't important */
1324+
null_expr= (Expr*)makeConst(INT4OID,
1325+
-1,
1326+
sizeof(int32),
1327+
(Datum)0,
1328+
true,/* isnull */
1329+
true/* byval */ );
1330+
newtlist=lappend(newtlist,
1331+
makeTargetEntry(null_expr,
1332+
colindex,
1333+
NULL,
1334+
false));
1335+
/* NULL insertion is dangerous in a setop */
1336+
if (parse->setOperations)
1337+
*modifyTargetList= true;
1338+
}
12501339
}
12511340

1252-
if (tlistlen!=tuplogcols)
1253-
ereport(ERROR,
1254-
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
1255-
errmsg("return type mismatch in function declared to return %s",
1256-
format_type_be(rettype)),
1257-
errdetail("Final statement returns too few columns.")));
1341+
if (modifyTargetList)
1342+
{
1343+
/* ensure resjunk columns are numbered correctly */
1344+
foreach(lc,junkattrs)
1345+
{
1346+
TargetEntry*tle= (TargetEntry*)lfirst(lc);
1347+
1348+
tle->resno=colindex++;
1349+
}
1350+
/* replace the tlist with the modified one */
1351+
*tlist_ptr=list_concat(newtlist,junkattrs);
1352+
}
12581353

12591354
/* Set up junk filter if needed */
12601355
if (junkFilter)

‎src/backend/optimizer/util/clauses.c‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.279 2009/10/08 02:39:21 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.280 2009/12/14 02:15:52 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHORDATEMAJOR EVENT
@@ -3673,6 +3673,7 @@ inline_function(Oid funcid, Oid result_type, List *args,
36733673
char*src;
36743674
Datumtmp;
36753675
boolisNull;
3676+
boolmodifyTargetList;
36763677
MemoryContextoldcxt;
36773678
MemoryContextmycxt;
36783679
ErrorContextCallbacksqlerrcontext;
@@ -3792,14 +3793,16 @@ inline_function(Oid funcid, Oid result_type, List *args,
37923793
* needed; that's probably not important, but let's be careful.
37933794
*/
37943795
if (check_sql_fn_retval(funcid,result_type,list_make1(querytree),
3795-
true,NULL))
3796+
&modifyTargetList,NULL))
37963797
gotofail;/* reject whole-tuple-result cases */
37973798

37983799
/* Now we can grab the tlist expression */
37993800
newexpr= (Node*) ((TargetEntry*)linitial(querytree->targetList))->expr;
38003801

38013802
/* Assert that check_sql_fn_retval did the right thing */
38023803
Assert(exprType(newexpr)==result_type);
3804+
/* It couldn't have made any dangerous tlist changes, either */
3805+
Assert(!modifyTargetList);
38033806

38043807
/*
38053808
* Additional validity checks on the expression. It mustn't return a set,
@@ -4088,6 +4091,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
40884091
char*src;
40894092
Datumtmp;
40904093
boolisNull;
4094+
boolmodifyTargetList;
40914095
MemoryContextoldcxt;
40924096
MemoryContextmycxt;
40934097
ErrorContextCallbacksqlerrcontext;
@@ -4253,20 +4257,28 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
42534257
* Make sure the function (still) returns what it's declared to. This
42544258
* will raise an error if wrong, but that's okay since the function would
42554259
* fail at runtime anyway.Note that check_sql_fn_retval will also insert
4256-
* RelabelType(s) if needed to make the tlist expression(s) match the
4257-
* declared type of the function.
4260+
* RelabelType(s)and/or NULL columnsif needed to make the tlist
4261+
*expression(s) match thedeclared type of the function.
42584262
*
42594263
* If the function returns a composite type, don't inline unless the check
42604264
* shows it's returning a whole tuple result; otherwise what it's
42614265
* returning is a single composite column which is not what we need.
42624266
*/
42634267
if (!check_sql_fn_retval(func_oid,fexpr->funcresulttype,
42644268
querytree_list,
4265-
true,NULL)&&
4269+
&modifyTargetList,NULL)&&
42664270
(get_typtype(fexpr->funcresulttype)==TYPTYPE_COMPOSITE||
42674271
fexpr->funcresulttype==RECORDOID))
42684272
gotofail;/* reject not-whole-tuple-result cases */
42694273

4274+
/*
4275+
* If we had to modify the tlist to make it match, and the statement is
4276+
* one in which changing the tlist contents could change semantics, we
4277+
* have to punt and not inline.
4278+
*/
4279+
if (modifyTargetList)
4280+
gotofail;
4281+
42704282
/*
42714283
* If it returns RECORD, we have to check against the column type list
42724284
* provided in the RTE; check_sql_fn_retval can't do that. (If no match,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp