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 63be2a44f16ea7755e55b3ab7be1f238d69427d2..5934ab029757f9d59fd49b5c8e966ec5ffcc43c2 100644 (file)
--- 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 cb07a066ef1054fc011ce503461ab9b62b8bf4c7..aab4897f618b01f0f1158ad2dd6a16d3481adb60 100644 (file)
--- 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 */