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