Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b97a346

Browse files
committed
Consider syntactic form when disambiguating function vs column reference.
Postgres has traditionally considered the syntactic forms f(x) and x.f to be equivalent, allowing tricks such as writing a function and then using it as though it were a computed-on-demand column. However, our behavior when both interpretations are feasible left something to be desired: we always chose the column interpretation. This could lead to very surprising results, as in a recent bug report from Neil Conway. It also created a dump-and-reload hazard, since what was a function call in a dumped view could get interpreted as a column reference at reload, if a matching column name had been added to the underlying table since the view was created. What seems better, in ambiguous situations, is to prefer the choice matching the syntactic form of the reference. This seems much less astonishing in general, and it fixes the dump/reload hazard. Although this could be called a bug fix, there have been few complaints and there's some small risk of breaking applications that depend on the old behavior, so no back-patch. It does seem reasonable to slip it into v11, though. Discussion: https://postgr.es/m/CAOW5sYa3Wp7KozCuzjOdw6PiOYPi6D=VvRybtH2S=2C0SVmRmA@mail.gmail.com
1 parent 4c8156d commit b97a346

File tree

4 files changed

+148
-50
lines changed

4 files changed

+148
-50
lines changed

doc/src/sgml/rowtypes.sgml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,12 @@ SELECT c.somefunc FROM inventory_item c;
441441
Because of this behavior, it's unwise to give a function that takes a
442442
single composite-type argument the same name as any of the fields of
443443
that composite type. If there is ambiguity, the field-name
444-
interpretation will be preferred, so that such a function could not be
445-
called without tricks. One way to force the function interpretation is
446-
to schema-qualify the function name, that is, write
444+
interpretation will be chosen if field-name syntax is used, while the
445+
function will be chosen if function-call syntax is used. However,
446+
<productname>PostgreSQL</productname> versions before 11 always chose the
447+
field-name interpretation, unless the syntax of the call required it to
448+
be a function call. One way to force the function interpretation in
449+
older versions is to schema-qualify the function name, that is, write
447450
<literal><replaceable>schema</replaceable>.<replaceable>func</replaceable>(<replaceable>compositevalue</replaceable>)</literal>.
448451
</para>
449452
</tip>

src/backend/parser/parse_func.c

Lines changed: 78 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,17 @@ static Node *ParseComplexProjection(ParseState *pstate, const char *funcname,
4949
* For historical reasons, Postgres tries to treat the notations tab.col
5050
* and col(tab) as equivalent: if a single-argument function call has an
5151
* argument of complex type and the (unqualified) function name matches
52-
* any attribute of the type, we take it as a column projection. Conversely
53-
* a function of a single complex-type argument can be written like a
54-
* column reference, allowing functions to act like computed columns.
52+
* any attribute of the type, we can interpret it as a column projection.
53+
* Conversely a function of a single complex-type argument can be written
54+
* like a column reference, allowing functions to act like computed columns.
55+
*
56+
* If both interpretations are possible, we prefer the one matching the
57+
* syntactic form, but otherwise the form does not matter.
5558
*
5659
* Hence, both cases come through here. If fn is null, we're dealing with
57-
* column syntax not function syntax, but in principle that should not
58-
* affect the lookup behavior, only which error messages we deliver.
59-
* The FuncCall struct is needed however to carry various decoration that
60-
* applies to aggregate and window functions.
60+
* column syntax not function syntax. In the function-syntax case,
61+
* the FuncCall struct is needed to carry various decoration that applies
62+
* to aggregate and window functions.
6163
*
6264
* Also, when fn is null, we return NULL on failure rather than
6365
* reporting a no-such-function error.
@@ -84,6 +86,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
8486
bool agg_distinct = (fn ? fn->agg_distinct : false);
8587
bool func_variadic = (fn ? fn->func_variadic : false);
8688
WindowDef *over = (fn ? fn->over : NULL);
89+
bool could_be_projection;
8790
Oid rettype;
8891
Oid funcid;
8992
ListCell *l;
@@ -202,36 +205,39 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
202205
}
203206

204207
/*
205-
* Check for column projection: if function has one argument, and that
206-
* argument is of complex type, and function name is not qualified, then
207-
* the "function call" could be a projection. We also check that there
208-
* wasn't any aggregate or variadic decoration, nor an argument name.
208+
* Decide whether it's legitimate to consider the construct to be a column
209+
* projection. For that, there has to be a single argument of complex
210+
* type, the function name must not be qualified, and there cannot be any
211+
* syntactic decoration that'd require it to be a function (such as
212+
* aggregate or variadic decoration, or named arguments).
209213
*/
210-
if (nargs == 1 && !proc_call &&
211-
agg_order == NIL && agg_filter == NULL && !agg_star &&
212-
!agg_distinct && over == NULL && !func_variadic && argnames == NIL &&
213-
list_length(funcname) == 1)
214-
{
215-
Oid argtype = actual_arg_types[0];
214+
could_be_projection = (nargs == 1 && !proc_call &&
215+
agg_order == NIL && agg_filter == NULL &&
216+
!agg_star && !agg_distinct && over == NULL &&
217+
!func_variadic && argnames == NIL &&
218+
list_length(funcname) == 1 &&
219+
(actual_arg_types[0] == RECORDOID ||
220+
ISCOMPLEX(actual_arg_types[0])));
216221

217-
if (argtype == RECORDOID || ISCOMPLEX(argtype))
218-
{
219-
retval = ParseComplexProjection(pstate,
220-
strVal(linitial(funcname)),
221-
first_arg,
222-
location);
223-
if (retval)
224-
return retval;
222+
/*
223+
* If it's column syntax, check for column projection case first.
224+
*/
225+
if (could_be_projection && is_column)
226+
{
227+
retval = ParseComplexProjection(pstate,
228+
strVal(linitial(funcname)),
229+
first_arg,
230+
location);
231+
if (retval)
232+
return retval;
225233

226-
/*
227-
* If ParseComplexProjection doesn't recognize it as a projection,
228-
* just press on.
229-
*/
230-
}
234+
/*
235+
* If ParseComplexProjection doesn't recognize it as a projection,
236+
* just press on.
237+
*/
231238
}
232239

233240
/*
234-
* Okay, it's not a column projection, so it must really be a function.
235241
* func_get_detail looks up the function in the catalogs, does
236242
* disambiguation for polymorphic functions, handles inheritance, and
237243
* returns the funcid and type and set or singleton status of the
@@ -334,7 +340,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
334340
}
335341

336342
/*
337-
* So far so good, so do some routine-type-specific processing.
343+
* So far so good, so do some fdresult-type-specific processing.
338344
*/
339345
if (fdresult == FUNCDETAIL_NORMAL || fdresult == FUNCDETAIL_PROCEDURE)
340346
{
@@ -524,30 +530,55 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
524530
actual_arg_types[0], rettype, -1,
525531
COERCION_EXPLICIT, COERCE_EXPLICIT_CALL, location);
526532
}
533+
else if (fdresult == FUNCDETAIL_MULTIPLE)
534+
{
535+
/*
536+
* We found multiple possible functional matches. If we are dealing
537+
* with attribute notation, return failure, letting the caller report
538+
* "no such column" (we already determined there wasn't one). If
539+
* dealing with function notation, report "ambiguous function",
540+
* regardless of whether there's also a column by this name.
541+
*/
542+
if (is_column)
543+
return NULL;
544+
545+
ereport(ERROR,
546+
(errcode(ERRCODE_AMBIGUOUS_FUNCTION),
547+
errmsg("function %s is not unique",
548+
func_signature_string(funcname, nargs, argnames,
549+
actual_arg_types)),
550+
errhint("Could not choose a best candidate function. "
551+
"You might need to add explicit type casts."),
552+
parser_errposition(pstate, location)));
553+
}
527554
else
528555
{
529556
/*
530-
* Oops. Time to die.
531-
*
532-
* If we are dealing with the attribute notation rel.function, let the
533-
* caller handle failure.
557+
* Not found as a function. If we are dealing with attribute
558+
* notation, return failure, letting the caller report "no such
559+
* column" (we already determined there wasn't one).
534560
*/
535561
if (is_column)
536562
return NULL;
537563

538564
/*
539-
* Else generate a detailed complaint for a function
565+
* Check for column projection interpretation, since we didn't before.
540566
*/
541-
if (fdresult == FUNCDETAIL_MULTIPLE)
542-
ereport(ERROR,
543-
(errcode(ERRCODE_AMBIGUOUS_FUNCTION),
544-
errmsg("function %s is not unique",
545-
func_signature_string(funcname, nargs, argnames,
546-
actual_arg_types)),
547-
errhint("Could not choose a best candidate function. "
548-
"You might need to add explicit type casts."),
549-
parser_errposition(pstate, location)));
550-
else if (list_length(agg_order) > 1 && !agg_within_group)
567+
if (could_be_projection)
568+
{
569+
retval = ParseComplexProjection(pstate,
570+
strVal(linitial(funcname)),
571+
first_arg,
572+
location);
573+
if (retval)
574+
return retval;
575+
}
576+
577+
/*
578+
* No function, and no column either. Since we're dealing with
579+
* function notation, report "function does not exist".
580+
*/
581+
if (list_length(agg_order) > 1 && !agg_within_group)
551582
{
552583
/* It's agg(x, ORDER BY y,z) ... perhaps misplaced ORDER BY */
553584
ereport(ERROR,

src/test/regress/expected/rowtypes.out

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,50 @@ select (row('Jim', 'Beam')).text; -- error
797797
ERROR: could not identify column "text" in record data type
798798
LINE 1: select (row('Jim', 'Beam')).text;
799799
^
800+
--
801+
-- Check the equivalence of functional and column notation
802+
--
803+
insert into fullname values ('Joe', 'Blow');
804+
select f.last from fullname f;
805+
last
806+
------
807+
Blow
808+
(1 row)
809+
810+
select last(f) from fullname f;
811+
last
812+
------
813+
Blow
814+
(1 row)
815+
816+
create function longname(fullname) returns text language sql
817+
as $$select $1.first || ' ' || $1.last$$;
818+
select f.longname from fullname f;
819+
longname
820+
----------
821+
Joe Blow
822+
(1 row)
823+
824+
select longname(f) from fullname f;
825+
longname
826+
----------
827+
Joe Blow
828+
(1 row)
829+
830+
-- Starting in v11, the notational form does matter if there's ambiguity
831+
alter table fullname add column longname text;
832+
select f.longname from fullname f;
833+
longname
834+
----------
835+
836+
(1 row)
837+
838+
select longname(f) from fullname f;
839+
longname
840+
----------
841+
Joe Blow
842+
(1 row)
843+
800844
--
801845
-- Test that composite values are seen to have the correct column names
802846
-- (bug #11210 and other reports)

src/test/regress/sql/rowtypes.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,26 @@ select (row('Jim', 'Beam'))::text;
345345
select text(row('Jim', 'Beam')); -- error
346346
select (row('Jim', 'Beam')).text; -- error
347347

348+
--
349+
-- Check the equivalence of functional and column notation
350+
--
351+
insert into fullname values ('Joe', 'Blow');
352+
353+
select f.last from fullname f;
354+
select last(f) from fullname f;
355+
356+
create function longname(fullname) returns text language sql
357+
as $$select $1.first || ' ' || $1.last$$;
358+
359+
select f.longname from fullname f;
360+
select longname(f) from fullname f;
361+
362+
-- Starting in v11, the notational form does matter if there's ambiguity
363+
alter table fullname add column longname text;
364+
365+
select f.longname from fullname f;
366+
select longname(f) from fullname f;
367+
348368
--
349369
-- Test that composite values are seen to have the correct column names
350370
-- (bug #11210 and other reports)

0 commit comments

Comments
 (0)