On Fri, Mar 3, 2017 at 4:12 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Wed, Mar 1, 2017 at 11:50 AM, Peter Eisentraut
> <peter.eisentr...@2ndquadrant.com> wrote:
>> This is the "grand finale" that goes on top of the "DROP FUNCTION of
>> multiple functions" patch set. The purpose is to allow referring to
>> functions without having to spell out the argument list, when the
>> function name is unique. This is especially useful when having to
>> operate on "business logic" functions with many many arguments. It's an
>> SQL standard feature, and it applies everywhere a function is referred
>> to in the grammar. We already have the lookup logic for the regproc
>> type, and thanks to the grand refactoring of the parser representation
>> of functions, this is quite a small patch. There is a bit of
>> reduce/reduce parser mystery, to keep the reviewer entertained.
>
> ... Which would be nice.
>
>> (The
>> equivalent could be implemented for aggregates and operators, but I
>> haven't done that here yet.)
>
> OK. After a lookup, I am just seeing opfamily, opclass missing, so
> this patch is doing it as you describe.
>
> I have read through the code once, still I am waiting for the DROP
> FUNCTION patches to be committed before doing a real hands-on.
>
> @@ -7198,6 +7198,33 @@ function_with_argtypes:
> n->objargs = extractArgTypes($2);
> $$ = n;
> }
> This may not have arguments listed, so is function_with_argtypes really
> adapted?
>
> + /*
> + * If no arguments were specified, the name must yield a unique candidate.
> + */
> + if (nargs == -1 && clist)
> + {
> + if (clist->next)
> + ereport(ERROR,
> I would have used list_length here for clarity.
>
> --- a/src/backend/parser/parse_func.c
> +++ b/src/backend/parser/parse_func.c
> @@ -1914,6 +1914,21 @@ LookupFuncName(List *funcname, int nargs, const
> Oid *argtypes, bool noError)
> The comment at the top of LookupFuncName() needs a refresh. The caller
> can as well just use a function name without arguments.
+ /*
+ * Because of reduce/reduce conflicts, we can't use func_name
+ * below, but we can write it out the long way, which actually
+ * allows more cases.
+ */
+ | type_func_name_keyword
+ {
+ ObjectWithArgs *n = makeNode(ObjectWithArgs);
+ n->objname = list_make1(makeString(pstrdup($1)));
+ n->args_unspecified = true;
+ $$ = n;
+ }
+ | ColId
+ {
+ ObjectWithArgs *n = makeNode(ObjectWithArgs);
+ n->objname = list_make1(makeString($1));
+ n->args_unspecified = true;
+ $$ = n;
+ }
+ | ColId indirection
+ {
+ ObjectWithArgs *n = makeNode(ObjectWithArgs);
+ n->objname = check_func_name(lcons(makeString($1), $2),
+ yyscanner);
+ n->args_unspecified = true;
+ $$ = n;
+ }
I have spent some time looking at this one. Another solution would be
to extend func_args to make the difference between an empty list and a
list with no arguments. This would need an intermediate structure for
parsing, and it does not seem worth the cost so I am fine with this
solution.
=# create schema popo;
CREATE SCHEMA
=# CREATE FUNCTION popo.dup2(int,int) RETURNS TABLE(f1 int, f2 text)
AS $$ SELECT
$1, CAST($1 AS text) || ' is text' $$
LANGUAGE SQL;
CREATE FUNCTION
=# CREATE FUNCTION dup2(int,int) RETURNS TABLE(f1 int, f2 text)
AS $$ SELECT
$1, CAST($1 AS text) || ' is text' $$
LANGUAGE SQL;
CREATE FUNCTION
=# set search_path to 'public,popo';
SET
Time: 0.463 ms
=# drop function dup2;
ERROR: 42883: function dup2() does not exist
LOCATION: LookupFuncName, parse_func.c:1944
In this case I would have expected an error telling that the name is
ambiguous. FuncnameGetCandidates() returns an empty list.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
