Re: [HACKERS] allow referring to functions without arguments when unique

[Peter Eisentraut]

On 3/14/17 03:03, Michael Paquier wrote:
> This looks good to me, so switched as ready for committer.

committed

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] allow referring to functions without arguments when unique

[Michael Paquier]

On Fri, Mar 10, 2017 at 2:03 PM, Peter Eisentraut
 wrote:
> On 3/7/17 00:32, Michael Paquier wrote:
>>> @@ -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?
>
> Well, we could do something like function_with_opt_argtypes?

I don't have a good idea about this name, so let's just keep it as-is...

>>> The comment at the top of LookupFuncName() needs a refresh. The caller
>>> can as well just use a function name without arguments.
>
> Fixed.
>
>> =# set search_path to 'public,popo';
>
> I think you mean
>
> set search_path to public,popo;

Thanks. Right.

>> =# 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.
>
> Your example works correctly if you set the schema path correctly.
> However, the error message is misleading with the parentheses.  I have
> updated that to create a different error message for this case, and
> added a test case.

+   {
+   if (!noError)
+   ereport(ERROR,
+   (errcode(ERRCODE_UNDEFINED_FUNCTION),
+errmsg("could not find a function named \"%s\"",
+   NameListToString(funcname;
+   }

Finally found out what I was looking for in the comments at the top of
FuncnameGetCandidates():
* We search a single namespace if the function name is qualified, else
 * all namespaces in the search path.  In the multiple-namespace case,
 * we arrange for entries in earlier namespaces to mask identical entries in
 * later namespaces.
And this correctly matches what your patch is doing, aka only the
first function found is removed and the next ones are discarded.

This looks good to me, so switched as ready for committer.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] allow referring to functions without arguments when unique

[Peter Eisentraut]

On 3/7/17 00:32, Michael Paquier wrote:
>> OK. After a lookup, I am just seeing opfamily, opclass missing, so
>> this patch is doing it as you describe.

I'm not sure what you mean here.

>> @@ -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?

Well, we could do something like function_with_opt_argtypes?

>> +   /*
>> +* 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.

This is actually not a "List" node.

>> The comment at the top of LookupFuncName() needs a refresh. The caller
>> can as well just use a function name without arguments.

Fixed.

> =# set search_path to 'public,popo';

I think you mean

set search_path to public,popo;

> =# 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.

Your example works correctly if you set the schema path correctly.
However, the error message is misleading with the parentheses.  I have
updated that to create a different error message for this case, and
added a test case.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From ac83b6f71433d2101f67dd148a4af2aac78129ee Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 9 Mar 2017 23:58:48 -0500
Subject: [PATCH v2] Allow referring to functions without arguments when unique

In DDL commands referring to an existing function, allow omitting the
argument list if the function name is unique in its schema, per SQL
standard.

This uses the same logic that the regproc type uses for finding
functions by name only.
---
 doc/src/sgml/ref/alter_extension.sgml   |  2 +-
 doc/src/sgml/ref/alter_function.sgml| 13 +
 doc/src/sgml/ref/alter_opfamily.sgml|  7 +++--
 doc/src/sgml/ref/comment.sgml   |  2 +-
 doc/src/sgml/ref/create_cast.sgml   |  6 ++--
 doc/src/sgml/ref/create_transform.sgml  | 12 +---
 doc/src/sgml/ref/drop_function.sgml | 35 ---
 doc/src/sgml/ref/grant.sgml |  2 +-
 doc/src/sgml/ref/revoke.sgml|  2 +-
 doc/src/sgml/ref/security_label.sgml|  2 +-
 src/backend/nodes/copyfuncs.c   |  1 +
 src/backend/nodes/equalfuncs.c  |  1 +
 src/backend/parser/gram.y   | 27 ++
 src/backend/parser/parse_func.c | 37 +++--
 src/include/nodes/parsenodes.h  |  3 ++
 src/test/regress/expected/create_function_3.out | 11 +++-
 src/test/regress/sql/create_function_3.sql  |  8 ++
 17 files changed, 143 insertions(+), 28 deletions(-)

diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml
index de6d6dca16..a7c0927d1c 100644
--- a/doc/src/sgml/ref/alter_extension.sgml
+++ b/doc/src/sgml/ref/alter_extension.sgml
@@ -39,7 +39,7 @@
   EVENT TRIGGER object_name |
   FOREIGN DATA WRAPPER object_name |
   FOREIGN TABLE object_name |
-  FUNCTION function_name ( [ [ argmode ] [ argname ] argtype [, ...] ] ) |
+  FUNCTION function_name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ] |
   MATERIALIZED VIEW object_name |
   OPERATOR operator_name (left_type, right_type) |
   OPERATOR CLASS object_name USING index_method |
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 0388d06b95..168eeb7c52 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -21,15 +21,15 @@
 
  
 
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 action [ ... ] [ RESTRICT ]
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 RENAME TO new_name
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 OWNER TO { new_owner | CURRENT_USER | SESSION_USER }
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 SET SCHEMA new_schema
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 DEPENDS ON EXTENSION extension_name
 
 where action is one 

Re: [HACKERS] allow referring to functions without arguments when unique

[Michael Paquier]

On Fri, Mar 3, 2017 at 4:12 PM, Michael Paquier
 wrote:
> On Wed, Mar 1, 2017 at 11:50 AM, Peter Eisentraut
>  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

Re: [HACKERS] allow referring to functions without arguments when unique

[Michael Paquier]

On Wed, Mar 1, 2017 at 11:50 AM, Peter Eisentraut
 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.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] allow referring to functions without arguments when unique

[Peter Eisentraut]

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.  (The
equivalent could be implemented for aggregates and operators, but I
haven't done that here yet.)

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 40eeb753abc83bbb0e38997fb518f9cd663f1729 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sat, 25 Feb 2017 19:13:15 -0500
Subject: [PATCH] Allow referring to functions without arguments when unique

In DDL commands referring to an existing function, allow omitting the
argument list if the function name is unique in its schema, per SQL
standard.

This uses the same logic that the regproc type uses for finding
functions by name only.
---
 doc/src/sgml/ref/alter_extension.sgml   |  2 +-
 doc/src/sgml/ref/alter_function.sgml| 13 ++--
 doc/src/sgml/ref/alter_opfamily.sgml|  7 ---
 doc/src/sgml/ref/comment.sgml   |  2 +-
 doc/src/sgml/ref/create_cast.sgml   |  6 --
 doc/src/sgml/ref/create_transform.sgml  | 12 +++
 doc/src/sgml/ref/drop_function.sgml | 26 
 doc/src/sgml/ref/grant.sgml |  2 +-
 doc/src/sgml/ref/revoke.sgml|  2 +-
 doc/src/sgml/ref/security_label.sgml|  2 +-
 src/backend/nodes/copyfuncs.c   |  1 +
 src/backend/nodes/equalfuncs.c  |  1 +
 src/backend/parser/gram.y   | 27 +
 src/backend/parser/parse_func.c | 17 +++-
 src/include/nodes/parsenodes.h  |  3 +++
 src/test/regress/expected/create_function_3.out |  9 -
 src/test/regress/sql/create_function_3.sql  |  7 +++
 17 files changed, 113 insertions(+), 26 deletions(-)

diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml
index de6d6dca16..a7c0927d1c 100644
--- a/doc/src/sgml/ref/alter_extension.sgml
+++ b/doc/src/sgml/ref/alter_extension.sgml
@@ -39,7 +39,7 @@
   EVENT TRIGGER object_name |
   FOREIGN DATA WRAPPER object_name |
   FOREIGN TABLE object_name |
-  FUNCTION function_name ( [ [ argmode ] [ argname ] argtype [, ...] ] ) |
+  FUNCTION function_name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ] |
   MATERIALIZED VIEW object_name |
   OPERATOR operator_name (left_type, right_type) |
   OPERATOR CLASS object_name USING index_method |
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 0388d06b95..168eeb7c52 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -21,15 +21,15 @@
 
  
 
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 action [ ... ] [ RESTRICT ]
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 RENAME TO new_name
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 OWNER TO { new_owner | CURRENT_USER | SESSION_USER }
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 SET SCHEMA new_schema
-ALTER FUNCTION name ( [ [ argmode ] [ argname ] argtype [, ...] ] )
+ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
 DEPENDS ON EXTENSION extension_name
 
 where action is one of:
@@ -75,7 +75,8 @@ Parameters
 name
 
  
-  The name (optionally schema-qualified) of an existing function.
+  The name (optionally schema-qualified) of an existing function.  If no
+  argument list is specified, the name must be unique in its schema.
  
 

diff --git a/doc/src/sgml/ref/alter_opfamily.sgml b/doc/src/sgml/ref/alter_opfamily.sgml
index 4511c7f7b2..0bafe5b8f8 100644
--- a/doc/src/sgml/ref/alter_opfamily.sgml
+++ b/doc/src/sgml/ref/alter_opfamily.sgml
@@ -25,7 +25,7 @@
   {  OPERATOR strategy_number operator_name ( op_type, op_type )
   [ FOR SEARCH | FOR ORDER BY sort_family_name ]
| FUNCTION support_number [ ( op_type [ , op_type ]