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