2014-10-11 22:28 GMT+07:00 Tom Lane <t...@sss.pgh.pa.us>: > Seems dangerous as heck; certainly it would have side-effects far more > wide-ranging than just making this particular function work. > > A safer answer is to split array_agg into two functions, > array_agg(anynonarray) -> anyarray > array_agg(anyarray) -> anyarray > > I rather imagine you should do that anyway, because I really doubt > that this hack is operating quite as intended. I suspect you are > producing arrays containing arrays as elements, not true 2-D arrays. > That's not a direction we want to go in I think; certainly there are > no other operations that produce such things. >
Thanks for the review. Yes, it looks like the patch produced array as the elements. So, all array operations behaves wierdly. In this quick & dirty patch, I am trying to implement the array_agg(anyarray), introducing two new functions: - array_agg_anyarray_transfn - array_agg_anyarray_finalfn At first, i want to use accumArrayResult and makeMdArrayResult, but it's complicated to work with multi-dimensional arrays with those two functions. So i combined array_cat with those function. Currently, it cannot handle NULL arrays: backend> select array_agg(a) from (values(null::int[])) a(a); 1: array_agg (typeid = 1007, len = -1, typmod = -1, byval = f) ---- ERROR: cannot aggregate null arrays Regards, -- Ali Akbar
*** a/src/backend/utils/adt/array_userfuncs.c --- b/src/backend/utils/adt/array_userfuncs.c *************** *** 16,21 **** --- 16,51 ---- #include "utils/builtins.h" #include "utils/lsyscache.h" + #include "utils/memutils.h" + + /*------------------------------------------------------------------------- + * ArrayAggAnyArrayState: + * aggregate state for array_agg(anyarray) + *------------------------------------------------------------------------- + */ + typedef struct + { + MemoryContext mcontext; /* where all the temp stuff is kept */ + char *data; /* array of accumulated data */ + bits8 *nullbitmap; /* bitmap of is-null flags for data */ + + int abytes; /* allocated length of above arrays */ + int aitems; /* allocated length of above arrays */ + int nbytes; /* number of used bytes in above arrays */ + int nitems; /* number of elements in above arrays */ + int narray; /* number of array accumulated */ + Oid element_type; /* data type of the Datums */ + int16 typlen; /* needed info about datatype */ + bool typbyval; + char typalign; + + int ndims; /* element dimensions */ + int *dims; + int *lbs; + + bool hasnull; /* any element has null */ + } ArrayAggAnyArrayState; + /*----------------------------------------------------------------------------- * array_push : *************** *** 544,546 **** array_agg_finalfn(PG_FUNCTION_ARGS) --- 574,814 ---- PG_RETURN_DATUM(result); } + + /* + * ARRAY_AGG(anyarray) aggregate function + */ + Datum + array_agg_anyarray_transfn(PG_FUNCTION_ARGS) + { + MemoryContext aggcontext, + arr_context, + oldcontext; + ArrayAggAnyArrayState *astate; + + Oid arg_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1); + Oid arg_elemtype = get_element_type(arg_typeid); + ArrayType *arg; + int *dims, + *lbs, + ndims, + nitems, + ndatabytes; + char *data; + + int i; + + if (arg_elemtype == InvalidOid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("could not determine input data type"))); + + if (!AggCheckCallContext(fcinfo, &aggcontext)) + elog(ERROR, "array_agg_anyarray_transfn called in non-aggregate context"); + + if (PG_ARGISNULL(1)) + elog(ERROR, "cannot aggregate null arrays"); + + astate = PG_ARGISNULL(0) ? NULL : (ArrayAggAnyArrayState *) PG_GETARG_POINTER(0); + + if (astate == NULL) + { + arr_context = AllocSetContextCreate(aggcontext, + "array_agg_anyarray_transfn", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); + oldcontext = MemoryContextSwitchTo(arr_context); + astate = (ArrayAggAnyArrayState *) palloc(sizeof(ArrayAggAnyArrayState)); + + astate->mcontext = arr_context; + astate->abytes = 0; + astate->aitems = 0; + astate->data = NULL; + astate->nullbitmap = NULL; + astate->nitems = 0; + astate->narray = 0; + astate->element_type = arg_elemtype; + get_typlenbyvalalign(arg_elemtype, + &astate->typlen, + &astate->typbyval, + &astate->typalign); + } + else + { + oldcontext = MemoryContextSwitchTo(astate->mcontext); + Assert(astate->element_type == arg_elemtype); + } + + arg = PG_GETARG_ARRAYTYPE_P(1); + + ndims = ARR_NDIM(arg); + dims = ARR_DIMS(arg); + lbs = ARR_LBOUND(arg); + data = ARR_DATA_PTR(arg); + nitems = ArrayGetNItems(ndims, dims); + + ndatabytes = ARR_SIZE(arg) - ARR_DATA_OFFSET(arg); + + if (astate->data == NULL) + { + if (ndims + 1 > MAXDIM) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", + ndims + 1, MAXDIM))); + + astate->ndims = ndims; + astate->dims = (int *) palloc(ndims * sizeof(int)); + astate->lbs = (int *) palloc(ndims * sizeof(int)); + memcpy(astate->dims, dims, ndims * sizeof(int)); + memcpy(astate->lbs, lbs, ndims * sizeof(int)); + + astate->abytes = 64 * (ndatabytes == 0 ? 1 : ndatabytes); + astate->aitems = 64 * nitems; + astate->data = (char *) palloc(astate->abytes); + + memcpy(astate->data, data, ndatabytes); + astate->nbytes = ndatabytes; + astate->nitems = nitems; + + if (ARR_HASNULL(arg)) + { + astate->hasnull = true; + astate->nullbitmap = (bits8 *) palloc((astate->aitems + 7) / 8); + array_bitmap_copy(astate->nullbitmap, 0, + ARR_NULLBITMAP(arg), 0, + nitems); + } + else + astate->hasnull = false; + } + else + { + if (astate->ndims != ndims) + ereport(ERROR, + (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), + errmsg("cannot aggregate incompatible arrays"), + errdetail("Arrays of %d and %d dimensions are not " + "compatible for concatenation.", + astate->ndims, ndims))); + + for (i = 0; i < ndims; i++) + if (astate->dims[i] != dims[i] || astate->lbs[i] != lbs[i]) + ereport(ERROR, + (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), + errmsg("cannot aggregate incompatible arrays"), + errdetail("Arrays with differing element dimensions are " + "not compatible for concatenation."))); + + if (astate->nbytes + ndatabytes >= astate->abytes) + { + astate->nbytes *= 2; + astate->data = (char *) + repalloc(astate->data, astate->nbytes); + } + if (astate->nitems + nitems >= astate->aitems) + { + astate->aitems *= 2; + astate->nullbitmap = (bits8 *) + repalloc(astate->nullbitmap, (astate->aitems + 7) / 8); + } + + memcpy(astate->data + astate->nbytes, data, ndatabytes); + astate->nbytes += ndatabytes; + + if (ARR_HASNULL(arg) || astate->hasnull) + { + if (!astate->hasnull) + { + astate->hasnull = true; + astate->nullbitmap = (bits8 *) palloc((astate->aitems + 7) / 8); + array_bitmap_copy(astate->nullbitmap, 0, + NULL, 0, + astate->nitems); + } + array_bitmap_copy(astate->nullbitmap, astate->nitems, + ARR_NULLBITMAP(arg), 0, + nitems); + astate->nitems += nitems; + } + } + astate->narray += 1; + + MemoryContextSwitchTo(oldcontext); + + /* + * The transition type for array_agg() is declared to be "internal", which + * is a pass-by-value type the same size as a pointer. So we can safely + * pass the pointer through nodeAgg.c's machinations. + */ + PG_RETURN_POINTER(astate); + } + + Datum + array_agg_anyarray_finalfn(PG_FUNCTION_ARGS) + { + MemoryContext oldcontext; + + ArrayType *result; + ArrayAggAnyArrayState *astate; + + int dataoffset, + nbytes, + ndims; + + /* + * Test for null before Asserting we are in right context. This is to + * avoid possible Assert failure in 8.4beta installations, where it is + * possible for users to create NULL constants of type internal. + */ + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); /* returns null iff no input values */ + + /* cannot be called directly because of internal-type argument */ + Assert(AggCheckCallContext(fcinfo, NULL)); + + oldcontext = MemoryContextSwitchTo(CurrentMemoryContext); + + astate = (ArrayAggAnyArrayState *) PG_GETARG_POINTER(0); + + ndims = astate->ndims + 1; + nbytes = astate->nbytes; + /* compute required space */ + if (astate->hasnull) + { + dataoffset = ARR_OVERHEAD_WITHNULLS(ndims, astate->nitems); + nbytes += dataoffset; + } + else + { + dataoffset = 0; + nbytes += ARR_OVERHEAD_NONULLS(ndims); + } + + result = (ArrayType *) palloc0(nbytes); + SET_VARSIZE(result, nbytes); + result->ndim = astate->ndims + 1; + result->dataoffset = dataoffset; + result->elemtype = astate->element_type; + + ARR_DIMS(result)[0] = astate->narray; + ARR_LBOUND(result)[0] = 1; + memcpy(&(ARR_DIMS(result)[1]), astate->dims, (ndims - 1) * sizeof(int)); + memcpy(&(ARR_LBOUND(result)[1]), astate->lbs, (ndims - 1) * sizeof(int)); + + memcpy(ARR_DATA_PTR(result), astate->data, astate->nbytes); + if (astate->hasnull) + array_bitmap_copy(ARR_NULLBITMAP(result), 0, + astate->nullbitmap, 0, + astate->nitems); + + MemoryContextSwitchTo(oldcontext); + + /* we cannot release ArrayAggAnyArrayState because sometimes + * aggregate final functions are re-executed. Rather, it is nodeAgg.c's + * responsibility to reset aggcontext when it's safe to do so + */ + + PG_RETURN_DATUM(PointerGetDatum(result)); + } *** a/src/include/catalog/pg_aggregate.h --- b/src/include/catalog/pg_aggregate.h *************** *** 275,280 **** DATA(insert ( 2901 n 0 xmlconcat2 - - - - f f 0 142 0 0 0 _null_ --- 275,281 ---- /* array */ DATA(insert ( 2335 n 0 array_agg_transfn array_agg_finalfn - - - t f 0 2281 0 0 0 _null_ _null_ )); + DATA(insert ( 6005 n 0 array_agg_anyarray_transfn array_agg_anyarray_finalfn - - - t f 0 2281 0 0 0 _null_ _null_ )); /* text */ DATA(insert ( 3538 n 0 string_agg_transfn string_agg_finalfn - - - f f 0 2281 0 0 0 _null_ _null_ )); *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *************** *** 879,889 **** DATA(insert OID = 3167 ( array_remove PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 DESCR("remove any occurrences of an element from an array"); DATA(insert OID = 3168 ( array_replace PGNSP PGUID 12 1 0 0 0 f f f f f f i 3 0 2277 "2277 2283 2283" _null_ _null_ _null_ _null_ array_replace _null_ _null_ _null_ )); DESCR("replace any occurrences of an element in an array"); ! DATA(insert OID = 2333 ( array_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 2283" _null_ _null_ _null_ _null_ array_agg_transfn _null_ _null_ _null_ )); DESCR("aggregate transition function"); ! DATA(insert OID = 2334 ( array_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2281 2283" _null_ _null_ _null_ _null_ array_agg_finalfn _null_ _null_ _null_ )); DESCR("aggregate final function"); ! DATA(insert OID = 2335 ( array_agg PGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 2277 "2283" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ )); DESCR("concatenate aggregate input into an array"); DATA(insert OID = 3218 ( width_bucket PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "2283 2277" _null_ _null_ _null_ _null_ width_bucket_array _null_ _null_ _null_ )); DESCR("bucket number of operand given a sorted array of bucket lower bounds"); --- 879,895 ---- DESCR("remove any occurrences of an element from an array"); DATA(insert OID = 3168 ( array_replace PGNSP PGUID 12 1 0 0 0 f f f f f f i 3 0 2277 "2277 2283 2283" _null_ _null_ _null_ _null_ array_replace _null_ _null_ _null_ )); DESCR("replace any occurrences of an element in an array"); ! DATA(insert OID = 2333 ( array_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 2776" _null_ _null_ _null_ _null_ array_agg_transfn _null_ _null_ _null_ )); DESCR("aggregate transition function"); ! DATA(insert OID = 2334 ( array_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2281 2776" _null_ _null_ _null_ _null_ array_agg_finalfn _null_ _null_ _null_ )); DESCR("aggregate final function"); ! DATA(insert OID = 2335 ( array_agg PGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 2277 "2776" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ )); ! DESCR("concatenate aggregate input into an array"); ! DATA(insert OID = 6003 ( array_agg_anyarray_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 2277" _null_ _null_ _null_ _null_ array_agg_anyarray_transfn _null_ _null_ _null_ )); ! DESCR("aggregate transition function"); ! DATA(insert OID = 6004 ( array_agg_anyarray_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2277 "2281 2277" _null_ _null_ _null_ _null_ array_agg_anyarray_finalfn _null_ _null_ _null_ )); ! DESCR("aggregate final function"); ! DATA(insert OID = 6005 ( array_agg PGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 2277 "2277" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ )); DESCR("concatenate aggregate input into an array"); DATA(insert OID = 3218 ( width_bucket PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "2283 2277" _null_ _null_ _null_ _null_ width_bucket_array _null_ _null_ _null_ )); DESCR("bucket number of operand given a sorted array of bucket lower bounds"); *** a/src/include/utils/array.h --- b/src/include/utils/array.h *************** *** 293,298 **** extern ArrayType *create_singleton_array(FunctionCallInfo fcinfo, --- 293,301 ---- extern Datum array_agg_transfn(PG_FUNCTION_ARGS); extern Datum array_agg_finalfn(PG_FUNCTION_ARGS); + extern Datum array_agg_anyarray_transfn(PG_FUNCTION_ARGS); + extern Datum array_agg_anyarray_finalfn(PG_FUNCTION_ARGS); + /* * prototypes for functions defined in array_typanalyze.c */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers