Split array_push into separate array_append and array_prepend functions.

There wasn't any good reason for a single C function to implement both
these SQL functions: it saved very little code overall, and it required
significant pushups to re-determine at runtime which case applied.  Redoing
it as two functions ends up with just slightly more lines of code, but it's
simpler to understand, and faster too because we need not repeat syscache
lookups on every call.

An important side benefit is that this eliminates the only case in which
different aliases of the same C function had both anyarray and anyelement
arguments at the same position, which would almost always be a mistake.
The opr_sanity regression test will now notice such mistakes since there's
no longer a valid case where it happens.

diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 600646e83492dbf009190a14f28225e272d1c42e..5c20d0c9d03c63aa61e9dfc4ae149a051a1917fc 100644 (file)
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -17,101 +17,151 @@
 #include "utils/lsyscache.h"
 
 
+/*
+ * fetch_array_arg_replace_nulls
+ *
+ * Fetch an array-valued argument; if it's null, construct an empty array
+ * value of the proper data type.  Also cache basic element type information
+ * in fn_extra.
+ */
+static ArrayType *
+fetch_array_arg_replace_nulls(FunctionCallInfo fcinfo, int argno)
+{
+   ArrayType  *v;
+   ArrayMetaState *my_extra;
+
+   my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
+   if (my_extra == NULL)
+   {
+       /* First time through, so look up the array type and element type */
+       Oid         arr_typeid = get_fn_expr_argtype(fcinfo->flinfo, argno);
+       Oid         element_type;
+
+       if (!OidIsValid(arr_typeid))
+           ereport(ERROR,
+                   (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                    errmsg("could not determine input data type")));
+       element_type = get_element_type(arr_typeid);
+       if (!OidIsValid(element_type))
+           ereport(ERROR,
+                   (errcode(ERRCODE_DATATYPE_MISMATCH),
+                    errmsg("input data type is not an array")));
+
+       my_extra = (ArrayMetaState *)
+           MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
+                              sizeof(ArrayMetaState));
+       my_extra->element_type = element_type;
+
+       /* Cache info about element type */
+       get_typlenbyvalalign(element_type,
+                            &my_extra->typlen,
+                            &my_extra->typbyval,
+                            &my_extra->typalign);
+
+       fcinfo->flinfo->fn_extra = my_extra;
+   }
+
+   /* Now we can collect the array value */
+   if (PG_ARGISNULL(argno))
+       v = construct_empty_array(my_extra->element_type);
+   else
+       v = PG_GETARG_ARRAYTYPE_P(argno);
+
+   return v;
+}
+
 /*-----------------------------------------------------------------------------
- * array_push :
- *     push an element onto either end of a one-dimensional array
+ * array_append :
+ *     push an element onto the end of a one-dimensional array
  *----------------------------------------------------------------------------
  */
 Datum
-array_push(PG_FUNCTION_ARGS)
+array_append(PG_FUNCTION_ARGS)
 {
    ArrayType  *v;
    Datum       newelem;
    bool        isNull;
+   ArrayType  *result;
    int        *dimv,
               *lb;
-   ArrayType  *result;
    int         indx;
-   Oid         element_type;
-   int16       typlen;
-   bool        typbyval;
-   char        typalign;
-   Oid         arg0_typeid = get_fn_expr_argtype(fcinfo->flinfo, 0);
-   Oid         arg1_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1);
-   Oid         arg0_elemid;
-   Oid         arg1_elemid;
    ArrayMetaState *my_extra;
 
-   if (arg0_typeid == InvalidOid || arg1_typeid == InvalidOid)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                errmsg("could not determine input data types")));
-
-   arg0_elemid = get_element_type(arg0_typeid);
-   arg1_elemid = get_element_type(arg1_typeid);
+   v = fetch_array_arg_replace_nulls(fcinfo, 0);
+   isNull = PG_ARGISNULL(1);
+   if (isNull)
+       newelem = (Datum) 0;
+   else
+       newelem = PG_GETARG_DATUM(1);
 
-   if (arg0_elemid != InvalidOid)
-   {
-       if (PG_ARGISNULL(0))
-           v = construct_empty_array(arg0_elemid);
-       else
-           v = PG_GETARG_ARRAYTYPE_P(0);
-       isNull = PG_ARGISNULL(1);
-       if (isNull)
-           newelem = (Datum) 0;
-       else
-           newelem = PG_GETARG_DATUM(1);
-   }
-   else if (arg1_elemid != InvalidOid)
+   if (ARR_NDIM(v) == 1)
    {
-       if (PG_ARGISNULL(1))
-           v = construct_empty_array(arg1_elemid);
-       else
-           v = PG_GETARG_ARRAYTYPE_P(1);
-       isNull = PG_ARGISNULL(0);
-       if (isNull)
-           newelem = (Datum) 0;
-       else
-           newelem = PG_GETARG_DATUM(0);
+       /* append newelem */
+       int         ub;
+
+       lb = ARR_LBOUND(v);
+       dimv = ARR_DIMS(v);
+       ub = dimv[0] + lb[0] - 1;
+       indx = ub + 1;
+
+       /* overflow? */
+       if (indx < ub)
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("integer out of range")));
    }
+   else if (ARR_NDIM(v) == 0)
+       indx = 1;
    else
-   {
-       /* Shouldn't get here given proper type checking in parser */
        ereport(ERROR,
-               (errcode(ERRCODE_DATATYPE_MISMATCH),
-                errmsg("neither input type is an array")));
-       PG_RETURN_NULL();       /* keep compiler quiet */
-   }
+               (errcode(ERRCODE_DATA_EXCEPTION),
+                errmsg("argument must be empty or one-dimensional array")));
+
+   /* Perform element insertion */
+   my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
 
-   element_type = ARR_ELEMTYPE(v);
+   result = array_set(v, 1, &indx, newelem, isNull,
+              -1, my_extra->typlen, my_extra->typbyval, my_extra->typalign);
+
+   PG_RETURN_ARRAYTYPE_P(result);
+}
+
+/*-----------------------------------------------------------------------------
+ * array_prepend :
+ *     push an element onto the front of a one-dimensional array
+ *----------------------------------------------------------------------------
+ */
+Datum
+array_prepend(PG_FUNCTION_ARGS)
+{
+   ArrayType  *v;
+   Datum       newelem;
+   bool        isNull;
+   ArrayType  *result;
+   int        *dimv,
+              *lb;
+   int         indx;
+   ArrayMetaState *my_extra;
+
+   isNull = PG_ARGISNULL(0);
+   if (isNull)
+       newelem = (Datum) 0;
+   else
+       newelem = PG_GETARG_DATUM(0);
+   v = fetch_array_arg_replace_nulls(fcinfo, 1);
 
    if (ARR_NDIM(v) == 1)
    {
+       /* prepend newelem */
        lb = ARR_LBOUND(v);
        dimv = ARR_DIMS(v);
+       indx = lb[0] - 1;
 
-       if (arg0_elemid != InvalidOid)
-       {
-           /* append newelem */
-           int         ub = dimv[0] + lb[0] - 1;
-
-           indx = ub + 1;
-           /* overflow? */
-           if (indx < ub)
-               ereport(ERROR,
-                       (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                        errmsg("integer out of range")));
-       }
-       else
-       {
-           /* prepend newelem */
-           indx = lb[0] - 1;
-           /* overflow? */
-           if (indx > lb[0])
-               ereport(ERROR,
-                       (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                        errmsg("integer out of range")));
-       }
+       /* overflow? */
+       if (indx > lb[0])
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("integer out of range")));
    }
    else if (ARR_NDIM(v) == 0)
        indx = 1;
@@ -120,39 +170,13 @@ array_push(PG_FUNCTION_ARGS)
                (errcode(ERRCODE_DATA_EXCEPTION),
                 errmsg("argument must be empty or one-dimensional array")));
 
-   /*
-    * We arrange to look up info about element type only once per series of
-    * calls, assuming the element type doesn't change underneath us.
-    */
+   /* Perform element insertion */
    my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
-   if (my_extra == NULL)
-   {
-       fcinfo->flinfo->fn_extra = MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
-                                                     sizeof(ArrayMetaState));
-       my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
-       my_extra->element_type = ~element_type;
-   }
-
-   if (my_extra->element_type != element_type)
-   {
-       /* Get info about element type */
-       get_typlenbyvalalign(element_type,
-                            &my_extra->typlen,
-                            &my_extra->typbyval,
-                            &my_extra->typalign);
-       my_extra->element_type = element_type;
-   }
-   typlen = my_extra->typlen;
-   typbyval = my_extra->typbyval;
-   typalign = my_extra->typalign;
 
    result = array_set(v, 1, &indx, newelem, isNull,
-                      -1, typlen, typbyval, typalign);
+              -1, my_extra->typlen, my_extra->typbyval, my_extra->typalign);
 
-   /*
-    * Readjust result's LB to match the input's.  This does nothing in the
-    * append case, but it's the simplest way to implement the prepend case.
-    */
+   /* Readjust result's LB to match the input's, as expected for prepend */
    if (ARR_NDIM(v) == 1)
        ARR_LBOUND(result)[0] = ARR_LBOUND(v)[0];
 

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 2b7a0bb93bc67fedf5233c987705b183f2df9f82..9100910c5e51999664fb77f78dfc5118b71a138f 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                         yyyymmddN */
-#define CATALOG_VERSION_NO 201501281
+#define CATALOG_VERSION_NO 201502181
 
 #endif

diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 9edfdb8fa89398e1940deff4111951e2b1a67297..7ddb4735647cbec12fe7f739c28df0e99ebc596a 100644 (file)
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -878,9 +878,9 @@ DATA(insert OID = 2176 (  array_length     PGNSP PGUID 12 1 0 0 0 f f f f t f i 2
 DESCR("array length");
 DATA(insert OID = 3179 (  cardinality     PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "2277" _null_ _null_ _null_ _null_ array_cardinality _null_ _null_ _null_ ));
 DESCR("array cardinality");
-DATA(insert OID = 378 (  array_append     PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2277 2283" _null_ _null_ _null_ _null_ array_push _null_ _null_ _null_ ));
+DATA(insert OID = 378 (  array_append     PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2277 2283" _null_ _null_ _null_ _null_ array_append _null_ _null_ _null_ ));
 DESCR("append element onto end of array");
-DATA(insert OID = 379 (  array_prepend    PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2283 2277" _null_ _null_ _null_ _null_ array_push _null_ _null_ _null_ ));
+DATA(insert OID = 379 (  array_prepend    PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2283 2277" _null_ _null_ _null_ _null_ array_prepend _null_ _null_ _null_ ));
 DESCR("prepend element onto front of array");
 DATA(insert OID = 383 (  array_cat        PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2277 2277" _null_ _null_ _null_ _null_ array_cat _null_ _null_ _null_ ));
 DATA(insert OID = 394 (  string_to_array   PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 1009 "25 25" _null_ _null_ _null_ _null_ text_to_array _null_ _null_ _null_ ));

diff --git a/src/include/utils/array.h b/src/include/utils/array.h
index dff69eba0659e3e7c3480d76b880c8dfbbffd215..d9fac807f86b0c771d728ca0ac26269fef0a327d 100644 (file)
--- a/src/include/utils/array.h
+++ b/src/include/utils/array.h
@@ -341,7 +341,8 @@ extern int32 *ArrayGetIntegerTypmods(ArrayType *arr, int *n);
 /*
  * prototypes for functions defined in array_userfuncs.c
  */
-extern Datum array_push(PG_FUNCTION_ARGS);
+extern Datum array_append(PG_FUNCTION_ARGS);
+extern Datum array_prepend(PG_FUNCTION_ARGS);
 extern Datum array_cat(PG_FUNCTION_ARGS);
 
 extern ArrayType *create_singleton_array(FunctionCallInfo fcinfo,

diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 9870bfaa0185bc9db5de600ab43e55d83172f9c1..6b248f2801af9c14ba651071ac0996defbca1f9e 100644 (file)
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -176,8 +176,7 @@ ORDER BY 1, 2;
           25 |        1043
         1114 |        1184
         1560 |        1562
-        2277 |        2283
-(5 rows)
+(4 rows)
 
 SELECT DISTINCT p1.proargtypes[1], p2.proargtypes[1]
 FROM pg_proc AS p1, pg_proc AS p2
@@ -194,8 +193,7 @@ ORDER BY 1, 2;
           23 |          28
         1114 |        1184
         1560 |        1562
-        2277 |        2283
-(4 rows)
+(3 rows)
 
 SELECT DISTINCT p1.proargtypes[2], p2.proargtypes[2]
 FROM pg_proc AS p1, pg_proc AS p2