From 741364bf5caeeae79b83bbdba778805d286622ba Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 3 Apr 2014 16:57:45 -0400
Subject: [PATCH] Code review for commit
d26888bc4d1e539a82f21382b0000fe5bbf889d9.
Mostly, copy-edit the comments; but also fix it to not reject domains over
arrays.
---
src/backend/parser/parse_func.c | 16 +++++++++-------
src/backend/utils/adt/varlena.c | 30 ++++++++++++++----------------
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 63be2a44f16..5934ab02975 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -559,7 +559,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
* If it's a variadic function call, transform the last nvargs arguments
* 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);
int non_var_args = nargs - nvargs;
@@ -587,19 +587,19 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
}
/*
- * When function is called with an explicit VARIADIC labeled parameter,
- * and the declared_arg_type is "any", then sanity check the actual
- * parameter type now - it must be an array.
+ * If an "any" variadic is called with explicit VARIADIC marking, insist
+ * that the variadic parameter be of some array type.
*/
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,
(errcode(ERRCODE_DATATYPE_MISMATCH),
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 */
@@ -1253,6 +1253,7 @@ func_get_detail(List *funcname,
*rettype = InvalidOid;
*retset = false;
*nvargs = 0;
+ *vatype = InvalidOid;
*true_typeids = NULL;
if (argdefaults)
*argdefaults = NIL;
@@ -1364,6 +1365,7 @@ func_get_detail(List *funcname,
*rettype = targetType;
*retset = false;
*nvargs = 0;
+ *vatype = InvalidOid;
*true_typeids = argtypes;
return FUNCDETAIL_COERCION;
}
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index cb07a066ef1..aab4897f618 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3834,16 +3834,15 @@ concat_internal(const char *sepstr, int argidx,
return NULL;
/*
- * Non-null argument had better be an array
- *
- * Correct values are ensured by parser check, but this function
- * can be called directly, bypassing the parser, so we should do
- * some minimal check too - this form of call requires correctly set
- * expr argtype in flinfo.
+ * 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
+ * checked that a VARIADIC-labeled parameter actually is an array. So
+ * it should be okay to just Assert that it's an array rather than
+ * doing a full-fledged error check.
*/
- Assert(OidIsValid(get_fn_expr_argtype(fcinfo->flinfo, argidx)));
- Assert(OidIsValid(get_element_type(get_fn_expr_argtype(fcinfo->flinfo, argidx))));
+ Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, argidx))));
+ /* OK, safe to fetch the array value */
arr = PG_GETARG_ARRAYTYPE_P(argidx);
/*
@@ -4063,16 +4062,15 @@ text_format(PG_FUNCTION_ARGS)
else
{
/*
- * Non-null argument had better be an array
- *
- * Correct values are ensured by parser check, but this function
- * can be called directly, bypassing the parser, so we should do
- * some minimal check too - this form of call requires correctly set
- * expr argtype in flinfo.
+ * 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 checked that a VARIADIC-labeled parameter actually is
+ * an array. So it should be okay to just Assert that it's an
+ * array rather than doing a full-fledged error check.
*/
- Assert(OidIsValid(get_fn_expr_argtype(fcinfo->flinfo, 1)));
- Assert(OidIsValid(get_element_type(get_fn_expr_argtype(fcinfo->flinfo, 1))));
+ Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 1))));
+ /* OK, safe to fetch the array value */
arr = PG_GETARG_ARRAYTYPE_P(1);
/* Get info about array element type */
--
GitLab