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
This commit is contained in:
Tom Lane 2018-06-16 14:10:17 -04:00
parent 15378c1a15
commit 0dcf68e5a1
2 changed files with 57 additions and 42 deletions

View File

@ -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)
/*
* 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)
{
/*
* 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?
* 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
{
/*

View File

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