Fix some minor error-checking oversights in ParseFuncOrColumn().

Recent additions to ParseFuncOrColumn to make it reject non-procedure
functions in CALL were neither adequate nor documented.  Reorganize
the code to ensure uniform results for all the cases that should be
rejected.  Also, use ERRCODE_WRONG_OBJECT_TYPE for this case as well
as the converse case of a procedure in a non-CALL context.  The
original coding used ERRCODE_UNDEFINED_FUNCTION which seems wrong,
and is certainly inconsistent with the adjacent wrong-kind-of-routine
errors.

This reorganization also causes the checks for aggregate decoration with
a non-aggregate function to be made in the FUNCDETAIL_COERCION case;
that they were not is a long-standing oversight.

Discussion: https://postgr.es/m/14497.1529089235@sss.pgh.pa.us

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index ea5d5212b4c86f9fcaca8afd27032ed0cf3704e7..21ddd5b7e01737fc8bb24ef455ef2ea0505cb0a6 100644 (file)
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -68,6 +68,9 @@ static Node *ParseComplexProjection(ParseState *pstate, const char *funcname,
  *     last_srf should be a copy of pstate->p_last_srf from just before we
  *     started transforming fargs.  If the caller knows that fargs couldn't
  *     contain any SRF calls, last_srf can just be pstate->p_last_srf.
+ *
+ *     proc_call is true if we are considering a CALL statement, so that the
+ *     name must resolve to a procedure name, not anything else.
  */
 Node *
 ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
@@ -204,7 +207,8 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
         * the "function call" could be a projection.  We also check that there
         * wasn't any aggregate or variadic decoration, nor an argument name.
         */
-       if (nargs == 1 && agg_order == NIL && agg_filter == NULL && !agg_star &&
+       if (nargs == 1 && !proc_call &&
+               agg_order == NIL && agg_filter == NULL && !agg_star &&
                !agg_distinct && over == NULL && !func_variadic && argnames == NIL &&
                list_length(funcname) == 1)
        {
@@ -253,21 +257,42 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 
        cancel_parser_errposition_callback(&pcbstate);
 
-       if (fdresult == FUNCDETAIL_COERCION)
-       {
-               /*
-                * We interpreted it as a type coercion. coerce_type can handle these
-                * cases, so why duplicate code...
-                */
-               return coerce_type(pstate, linitial(fargs),
-                                                  actual_arg_types[0], rettype, -1,
-                                                  COERCION_EXPLICIT, COERCE_EXPLICIT_CALL, location);
-       }
-       else if (fdresult == FUNCDETAIL_NORMAL || fdresult == FUNCDETAIL_PROCEDURE)
+       /*
+        * Check for various wrong-kind-of-routine cases.
+        */
+
+       /* If this is a CALL, reject things that aren't procedures */
+       if (proc_call &&
+               (fdresult == FUNCDETAIL_NORMAL ||
+                fdresult == FUNCDETAIL_AGGREGATE ||
+                fdresult == FUNCDETAIL_WINDOWFUNC ||
+                fdresult == FUNCDETAIL_COERCION))
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("%s is not a procedure",
+                                               func_signature_string(funcname, nargs,
+                                                                                         argnames,
+                                                                                         actual_arg_types)),
+                                errhint("To call a function, use SELECT."),
+                                parser_errposition(pstate, location)));
+       /* Conversely, if not a CALL, reject procedures */
+       if (fdresult == FUNCDETAIL_PROCEDURE && !proc_call)
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("%s is a procedure",
+                                               func_signature_string(funcname, nargs,
+                                                                                         argnames,
+                                                                                         actual_arg_types)),
+                                errhint("To call a procedure, use CALL."),
+                                parser_errposition(pstate, location)));
+
+       if (fdresult == FUNCDETAIL_NORMAL ||
+               fdresult == FUNCDETAIL_PROCEDURE ||
+               fdresult == FUNCDETAIL_COERCION)
        {
                /*
-                * Normal function found; was there anything indicating it must be an
-                * aggregate?
+                * In these cases, complain if there was anything indicating it must
+                * be an aggregate or window function.
                 */
                if (agg_star)
                        ereport(ERROR,
@@ -306,26 +331,14 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                                         errmsg("OVER specified, but %s is not a window function nor an aggregate function",
                                                        NameListToString(funcname)),
                                         parser_errposition(pstate, location)));
+       }
 
-               if (fdresult == FUNCDETAIL_NORMAL && proc_call)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_FUNCTION),
-                                        errmsg("%s is not a procedure",
-                                                       func_signature_string(funcname, nargs,
-                                                                                                 argnames,
-                                                                                                 actual_arg_types)),
-                                        errhint("To call a function, use SELECT."),
-                                        parser_errposition(pstate, location)));
-
-               if (fdresult == FUNCDETAIL_PROCEDURE && !proc_call)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_FUNCTION),
-                                        errmsg("%s is a procedure",
-                                                       func_signature_string(funcname, nargs,
-                                                                                                 argnames,
-                                                                                                 actual_arg_types)),
-                                        errhint("To call a procedure, use CALL."),
-                                        parser_errposition(pstate, location)));
+       /*
+        * So far so good, so do some routine-type-specific processing.
+        */
+       if (fdresult == FUNCDETAIL_NORMAL || fdresult == FUNCDETAIL_PROCEDURE)
+       {
+               /* Nothing special to do for these cases. */
        }
        else if (fdresult == FUNCDETAIL_AGGREGATE)
        {
@@ -336,15 +349,6 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                Form_pg_aggregate classForm;
                int                     catDirectArgs;
 
-               if (proc_call)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_FUNCTION),
-                                        errmsg("%s is not a procedure",
-                                                       func_signature_string(funcname, nargs,
-                                                                                                 argnames,
-                                                                                                 actual_arg_types)),
-                                        parser_errposition(pstate, location)));
-
                tup = SearchSysCache1(AGGFNOID, ObjectIdGetDatum(funcid));
                if (!HeapTupleIsValid(tup)) /* should not happen */
                        elog(ERROR, "cache lookup failed for aggregate %u", funcid);
@@ -510,6 +514,16 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                                                        NameListToString(funcname)),
                                         parser_errposition(pstate, location)));
        }
+       else if (fdresult == FUNCDETAIL_COERCION)
+       {
+               /*
+                * We interpreted it as a type coercion. coerce_type can handle these
+                * cases, so why duplicate code...
+                */
+               return coerce_type(pstate, linitial(fargs),
+                                                  actual_arg_types[0], rettype, -1,
+                                                  COERCION_EXPLICIT, COERCE_EXPLICIT_CALL, location);
+       }
        else
        {
                /*

diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out
index 67d671727c7708eb9551c50e1231f3b0eb2d26fa..30495971bc8be540a45f1f781e893930efd75cd9 100644 (file)
--- a/src/test/regress/expected/create_procedure.out
+++ b/src/test/regress/expected/create_procedure.out
@@ -126,6 +126,7 @@ CALL sum(1);  -- error: not a procedure
 ERROR:  sum(integer) is not a procedure
 LINE 1: CALL sum(1);
              ^
+HINT:  To call a function, use SELECT.
 CREATE PROCEDURE ptestx() LANGUAGE SQL WINDOW AS $$ INSERT INTO cp_test VALUES (1, 'a') $$;
 ERROR:  invalid attribute in procedure definition
 LINE 1: CREATE PROCEDURE ptestx() LANGUAGE SQL WINDOW AS $$ INSERT I...