Commit 741364bf authored by Tom Lane's avatar Tom Lane

Code review for commit d26888bc.

Mostly, copy-edit the comments; but also fix it to not reject domains over
arrays.
parent 42c6236f
...@@ -559,7 +559,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, ...@@ -559,7 +559,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
* If it's a variadic function call, transform the last nvargs arguments * If it's a variadic function call, transform the last nvargs arguments
* into an array --- unless it's an "any" variadic. * into an array --- unless it's an "any" variadic.
*/ */
if (nvargs > 0 && declared_arg_types[nargs - 1] != ANYOID) if (nvargs > 0 && vatype != ANYOID)
{ {
ArrayExpr *newa = makeNode(ArrayExpr); ArrayExpr *newa = makeNode(ArrayExpr);
int non_var_args = nargs - nvargs; int non_var_args = nargs - nvargs;
...@@ -587,19 +587,19 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, ...@@ -587,19 +587,19 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
} }
/* /*
* When function is called with an explicit VARIADIC labeled parameter, * If an "any" variadic is called with explicit VARIADIC marking, insist
* and the declared_arg_type is "any", then sanity check the actual * that the variadic parameter be of some array type.
* parameter type now - it must be an array.
*/ */
if (nargs > 0 && vatype == ANYOID && func_variadic) if (nargs > 0 && vatype == ANYOID && func_variadic)
{ {
Oid va_arr_typid = actual_arg_types[nargs - 1]; Oid va_arr_typid = actual_arg_types[nargs - 1];
if (!OidIsValid(get_element_type(va_arr_typid))) if (!OidIsValid(get_base_element_type(va_arr_typid)))
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH), (errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("VARIADIC argument must be an array"), errmsg("VARIADIC argument must be an array"),
parser_errposition(pstate, exprLocation((Node *) llast(fargs))))); parser_errposition(pstate,
exprLocation((Node *) llast(fargs)))));
} }
/* build the appropriate output structure */ /* build the appropriate output structure */
...@@ -1253,6 +1253,7 @@ func_get_detail(List *funcname, ...@@ -1253,6 +1253,7 @@ func_get_detail(List *funcname,
*rettype = InvalidOid; *rettype = InvalidOid;
*retset = false; *retset = false;
*nvargs = 0; *nvargs = 0;
*vatype = InvalidOid;
*true_typeids = NULL; *true_typeids = NULL;
if (argdefaults) if (argdefaults)
*argdefaults = NIL; *argdefaults = NIL;
...@@ -1364,6 +1365,7 @@ func_get_detail(List *funcname, ...@@ -1364,6 +1365,7 @@ func_get_detail(List *funcname,
*rettype = targetType; *rettype = targetType;
*retset = false; *retset = false;
*nvargs = 0; *nvargs = 0;
*vatype = InvalidOid;
*true_typeids = argtypes; *true_typeids = argtypes;
return FUNCDETAIL_COERCION; return FUNCDETAIL_COERCION;
} }
......
...@@ -3834,16 +3834,15 @@ concat_internal(const char *sepstr, int argidx, ...@@ -3834,16 +3834,15 @@ concat_internal(const char *sepstr, int argidx,
return NULL; return NULL;
/* /*
* Non-null argument had better be an array * Non-null argument had better be an array. We assume that any call
* * context that could let get_fn_expr_variadic return true will have
* Correct values are ensured by parser check, but this function * checked that a VARIADIC-labeled parameter actually is an array. So
* can be called directly, bypassing the parser, so we should do * it should be okay to just Assert that it's an array rather than
* some minimal check too - this form of call requires correctly set * doing a full-fledged error check.
* expr argtype in flinfo.
*/ */
Assert(OidIsValid(get_fn_expr_argtype(fcinfo->flinfo, argidx))); Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, argidx))));
Assert(OidIsValid(get_element_type(get_fn_expr_argtype(fcinfo->flinfo, argidx))));
/* OK, safe to fetch the array value */
arr = PG_GETARG_ARRAYTYPE_P(argidx); arr = PG_GETARG_ARRAYTYPE_P(argidx);
/* /*
...@@ -4063,16 +4062,15 @@ text_format(PG_FUNCTION_ARGS) ...@@ -4063,16 +4062,15 @@ text_format(PG_FUNCTION_ARGS)
else else
{ {
/* /*
* Non-null argument had better be an array * Non-null argument had better be an array. We assume that any
* * call context that could let get_fn_expr_variadic return true
* Correct values are ensured by parser check, but this function * will have checked that a VARIADIC-labeled parameter actually is
* can be called directly, bypassing the parser, so we should do * an array. So it should be okay to just Assert that it's an
* some minimal check too - this form of call requires correctly set * array rather than doing a full-fledged error check.
* expr argtype in flinfo.
*/ */
Assert(OidIsValid(get_fn_expr_argtype(fcinfo->flinfo, 1))); Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 1))));
Assert(OidIsValid(get_element_type(get_fn_expr_argtype(fcinfo->flinfo, 1))));
/* OK, safe to fetch the array value */
arr = PG_GETARG_ARRAYTYPE_P(1); arr = PG_GETARG_ARRAYTYPE_P(1);
/* Get info about array element type */ /* Get info about array element type */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment