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 */