From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 3 Apr 2014 20:57:45 +0000 (-0400)
Subject: Code review for commit d26888bc4d1e539a82f21382b0000fe5bbf889d9.
X-Git-Tag: REL9_4_BETA1~236
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=741364bf5caeeae79b83bbdba778805d286622ba;p=postgresql

Code review for commit d26888bc4d1e539a82f21382b0000fe5bbf889d9.

Mostly, copy-edit the comments; but also fix it to not reject domains over
arrays.
---

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 63be2a44f1..5934ab0297 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 cb07a066ef..aab4897f61 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 */