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

Commitc7b3539

Browse files
committed
Fix non-equivalence of VARIADIC and non-VARIADIC function call formats.
For variadic functions (other than VARIADIC ANY), the syntaxes foo(x,y,...)and foo(VARIADIC ARRAY[x,y,...]) should be considered equivalent, since theformer is converted to the latter at parse time. They have indeed beenequivalent, in all releases before 9.3. However, commit75b39e7 made anill-considered decision to record which syntax had been used in FuncExprnodes, and then to make equal() test that in checking node equality ---which caused the syntaxes to not be seen as equivalent by the planner.This is the underlying cause of bug #9817 from Dmitry Ryabov.It might seem that a quick fix would be to make equal() disregardFuncExpr.funcvariadic, but the same commit made that untenable, becausethe field actually *is* semantically significant for some VARIADIC ANYfunctions. This patch instead adopts the approach of redefiningfuncvariadic (and aggvariadic, in HEAD) as meaning that the last argumentis a variadic array, whether it got that way by parser intervention or wassupplied explicitly by the user. Therefore the value will always be truefor non-ANY variadic functions, restoring the principle of equivalence.(However, the planner will continue to consider use of VARIADIC as ameaningful difference for VARIADIC ANY functions, even though some suchfunctions might disregard it.)In HEAD, this change lets us simplify the decompilation logic inruleutils.c, since the funcvariadic/aggvariadic flag tells directly whetherto print VARIADIC. However, in 9.3 we have to continue to cope withexisting stored rules/views that might contain the previous definition.Fortunately, this just means no change in ruleutils.c, since its existingbehavior effectively ignores funcvariadic for all cases other than VARIADICANY functions.In HEAD, bump catversion to reflect the fact that FuncExpr.funcvariadicchanged meanings; this is sort of pro forma, since I don't believe anybuilt-in views are affected.Unfortunately, this patch doesn't magically fix everything for affected9.3 users. After installing 9.3.5, they might need to recreate theirrules/views/indexes containing variadic function calls in order to geteverything consistent with the new definition. As in the cited bug,the symptom of a problem would be failure to use a nominally matchingindex that has a variadic function call in its definition. We'll needto mention this in the 9.3.5 release notes.
1 parent741364b commitc7b3539

File tree

7 files changed

+46
-38
lines changed

7 files changed

+46
-38
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,15 +1548,7 @@ deparseFuncExpr(FuncExpr *node, deparse_expr_cxt *context)
15481548
procform= (Form_pg_proc)GETSTRUCT(proctup);
15491549

15501550
/* Check if need to print VARIADIC (cf. ruleutils.c) */
1551-
if (OidIsValid(procform->provariadic))
1552-
{
1553-
if (procform->provariadic!=ANYOID)
1554-
use_variadic= true;
1555-
else
1556-
use_variadic=node->funcvariadic;
1557-
}
1558-
else
1559-
use_variadic= false;
1551+
use_variadic=node->funcvariadic;
15601552

15611553
/* Print schema name only if it's not pg_catalog */
15621554
if (procform->pronamespace!=PG_CATALOG_NAMESPACE)

‎doc/src/sgml/xfunc.sgml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3152,9 +3152,10 @@ CREATE OR REPLACE FUNCTION retcomposite(IN integer, IN integer,
31523152
is zero based. <function>get_call_result_type</> can also be used
31533153
as an alternative to <function>get_fn_expr_rettype</>.
31543154
There is also <function>get_fn_expr_variadic</>, which can be used to
3155-
find out whether the call contained an explicit <literal>VARIADIC</>
3156-
keyword. This is primarily useful for <literal>VARIADIC "any"</>
3157-
functions, as described below.
3155+
find out whether variadic arguments have been merged into an array.
3156+
This is primarily useful for <literal>VARIADIC "any"</> functions,
3157+
since such merging will always have occurred for variadic functions
3158+
taking ordinary array types.
31583159
</para>
31593160

31603161
<para>

‎src/backend/parser/parse_func.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,17 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
555555
/* perform the necessary typecasting of arguments */
556556
make_fn_arguments(pstate,fargs,actual_arg_types,declared_arg_types);
557557

558+
/*
559+
* If the function isn't actually variadic, forget any VARIADIC decoration
560+
* on the call. (Perhaps we should throw an error instead, but
561+
* historically we've allowed people to write that.)
562+
*/
563+
if (!OidIsValid(vatype))
564+
{
565+
Assert(nvargs==0);
566+
func_variadic= false;
567+
}
568+
558569
/*
559570
* If it's a variadic function call, transform the last nvargs arguments
560571
* into an array --- unless it's an "any" variadic.
@@ -584,6 +595,11 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
584595
newa->location=exprLocation((Node*)vargs);
585596

586597
fargs=lappend(fargs,newa);
598+
599+
/* We could not have had VARIADIC marking before ... */
600+
Assert(!func_variadic);
601+
/* ... but now, it's a VARIADIC call */
602+
func_variadic= true;
587603
}
588604

589605
/*

‎src/backend/utils/adt/ruleutils.c

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ static char *get_relation_name(Oid relid);
401401
staticchar*generate_relation_name(Oidrelid,List*namespaces);
402402
staticchar*generate_function_name(Oidfuncid,intnargs,
403403
List*argnames,Oid*argtypes,
404-
boolwas_variadic,bool*use_variadic_p);
404+
boolhas_variadic,bool*use_variadic_p);
405405
staticchar*generate_operator_name(Oidoperid,Oidarg1,Oidarg2);
406406
statictext*string_to_text(char*str);
407407
staticchar*flatten_reloptions(Oidrelid);
@@ -8849,16 +8849,16 @@ generate_relation_name(Oid relid, List *namespaces)
88498849
*
88508850
* If we're dealing with a potentially variadic function (in practice, this
88518851
* means a FuncExpr or Aggref, not some other way of calling a function), then
8852-
*was_variadic must specify whetherVARIADIC appeared in the original call,
8852+
*has_variadic must specify whethervariadic arguments have been merged,
88538853
* and *use_variadic_p will be set to indicate whether to print VARIADIC in
8854-
* the output.For non-FuncExpr cases,was_variadic should be FALSE and
8854+
* the output.For non-FuncExpr cases,has_variadic should be FALSE and
88558855
* use_variadic_p can be NULL.
88568856
*
88578857
* The result includes all necessary quoting and schema-prefixing.
88588858
*/
88598859
staticchar*
88608860
generate_function_name(Oidfuncid,intnargs,List*argnames,Oid*argtypes,
8861-
boolwas_variadic,bool*use_variadic_p)
8861+
boolhas_variadic,bool*use_variadic_p)
88628862
{
88638863
char*result;
88648864
HeapTupleproctup;
@@ -8884,32 +8884,27 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes,
88848884
* Determine whether VARIADIC should be printed. We must do this first
88858885
* since it affects the lookup rules in func_get_detail().
88868886
*
8887-
* Currently, we always print VARIADIC if the function is variadic and
8888-
* takes a variadic type other than ANY. (In principle, if VARIADIC
8889-
* wasn't originally specified and the array actual argument is
8890-
* deconstructable, we could print the array elements separately and not
8891-
* print VARIADIC, thus more nearly reproducing the original input. For
8892-
* the moment that seems like too much complication for the benefit.)
8893-
* However, if the function takes VARIADIC ANY, then the parser didn't
8894-
* fold the arguments together into an array, so we must print VARIADIC if
8895-
* and only if it was used originally.
8887+
* Currently, we always print VARIADIC if the function has a merged
8888+
* variadic-array argument. Note that this is always the case for
8889+
* functions taking a VARIADIC argument type other than VARIADIC ANY.
8890+
*
8891+
* In principle, if VARIADIC wasn't originally specified and the array
8892+
* actual argument is deconstructable, we could print the array elements
8893+
* separately and not print VARIADIC, thus more nearly reproducing the
8894+
* original input. For the moment that seems like too much complication
8895+
* for the benefit, and anyway we do not know whether VARIADIC was
8896+
* originally specified if it's a non-ANY type.
88968897
*/
88978898
if (use_variadic_p)
88988899
{
8899-
if (OidIsValid(procform->provariadic))
8900-
{
8901-
if (procform->provariadic!=ANYOID)
8902-
use_variadic= true;
8903-
else
8904-
use_variadic=was_variadic;
8905-
}
8906-
else
8907-
use_variadic= false;
8900+
/* Parser should not have set funcvariadic unless fn is variadic */
8901+
Assert(!has_variadic||OidIsValid(procform->provariadic));
8902+
use_variadic=has_variadic;
89088903
*use_variadic_p=use_variadic;
89098904
}
89108905
else
89118906
{
8912-
Assert(!was_variadic);
8907+
Assert(!has_variadic);
89138908
use_variadic= false;
89148909
}
89158910

‎src/backend/utils/fmgr/fmgr.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2449,6 +2449,8 @@ get_call_expr_arg_stable(Node *expr, int argnum)
24492449
* Get the VARIADIC flag from the function invocation
24502450
*
24512451
* Returns false (the default assumption) if information is not available
2452+
*
2453+
* Note this is generally only of interest to VARIADIC ANY functions
24522454
*/
24532455
bool
24542456
get_fn_expr_variadic(FmgrInfo*flinfo)

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO201403261
56+
#defineCATALOG_VERSION_NO201404031
5757

5858
#endif

‎src/include/nodes/primnodes.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,8 @@ typedef struct Aggref
257257
List*aggdistinct;/* DISTINCT (list of SortGroupClause) */
258258
Expr*aggfilter;/* FILTER expression, if any */
259259
boolaggstar;/* TRUE if argument list was really '*' */
260-
boolaggvariadic;/* TRUE if VARIADIC was used in call */
260+
boolaggvariadic;/* true if variadic arguments have been
261+
* combined into an array last argument */
261262
charaggkind;/* aggregate kind (see pg_aggregate.h) */
262263
Indexagglevelsup;/* > 0 if agg belongs to outer query */
263264
intlocation;/* token location, or -1 if unknown */
@@ -358,7 +359,8 @@ typedef struct FuncExpr
358359
Oidfuncid;/* PG_PROC OID of the function */
359360
Oidfuncresulttype;/* PG_TYPE OID of result value */
360361
boolfuncretset;/* true if function returns set */
361-
boolfuncvariadic;/* true if VARIADIC was used in call */
362+
boolfuncvariadic;/* true if variadic arguments have been
363+
* combined into an array last argument */
362364
CoercionFormfuncformat;/* how to display this function call */
363365
Oidfunccollid;/* OID of collation of result */
364366
Oidinputcollid;/* OID of collation that function should use */

0 commit comments

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