Procedure additions to ParseFuncOrColumn are inadequate

I noticed that ParseFuncOrColumn isn't terribly thorough about rejecting
non-procedure results when proc_call is true.  Since the caller is just
assuming that it gets back what it expects, that leads to core dumps
or worse:

regression=# call int4(42);
server closed the connection unexpectedly
        This probably means the server terminated abnormally

Also, with the existing ad-hoc approach to this, not all the ereports
are alike; the one for an aggregate lacks the HINT the others provide.

Also, in the cases where it does successfully reject should-be-procedure
or should-not-be-procedure, it reports ERRCODE_UNDEFINED_FUNCTION, which
seems quite inappropriate from here: I'd expect ERRCODE_WRONG_OBJECT_TYPE.

There's also a pre-existing oversight that when we get a result of
FUNCDETAIL_COERCION, we should reject that if there's any aggregate
decoration, but we fail to.

The attached cleans all this up; any objections?

            regards, tom lane

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index ea5d521..21ddd5b 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*************** static Node *ParseComplexProjection(Pars
*** 68,73 ****
--- 68,76 ----
   *    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,
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 204,210 ****
       * 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 &&
          !agg_distinct && over == NULL && !func_variadic && argnames == NIL &&
          list_length(funcname) == 1)
      {
--- 207,214 ----
       * 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 && !proc_call &&
!         agg_order == NIL && agg_filter == NULL && !agg_star &&
          !agg_distinct && over == NULL && !func_variadic && argnames == NIL &&
          list_length(funcname) == 1)
      {
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 253,273 ****

      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)
      {
          /*
!          * Normal function found; was there anything indicating it must be an
!          * aggregate?
           */
          if (agg_star)
              ereport(ERROR,
--- 257,298 ----

      cancel_parser_errposition_callback(&pcbstate);

!     /*
!      * 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)
      {
          /*
!          * In these cases, complain if there was anything indicating it must
!          * be an aggregate or window function.
           */
          if (agg_star)
              ereport(ERROR,
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 306,331 ****
                       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)));
      }
      else if (fdresult == FUNCDETAIL_AGGREGATE)
      {
--- 331,344 ----
                       errmsg("OVER specified, but %s is not a window function nor an aggregate function",
                              NameListToString(funcname)),
                       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)
      {
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 336,350 ****
          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);
--- 349,354 ----
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 510,515 ****
--- 514,529 ----
                              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 67d6717..3049597 100644
*** a/src/test/regress/expected/create_procedure.out
--- b/src/test/regress/expected/create_procedure.out
*************** CALL sum(1);  -- error: not a procedure
*** 126,131 ****
--- 126,132 ----
  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...