diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index d9faf20..9c97755 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -262,7 +262,7 @@ ExecScanSubPlan(SubPlanState *node, /* Initialize ArrayBuildStateAny in caller's context, if needed */ if (subLinkType == ARRAY_SUBLINK) astate = initArrayResultAny(subplan->firstColType, - CurrentMemoryContext); + CurrentMemoryContext, true); /* * We are probably in a short-lived expression-evaluation context. Switch @@ -964,7 +964,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) /* Initialize ArrayBuildStateAny in caller's context, if needed */ if (subLinkType == ARRAY_SUBLINK) astate = initArrayResultAny(subplan->firstColType, - CurrentMemoryContext); + CurrentMemoryContext, true); /* * Must switch to per-query memory context. diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index 50ea4d2..f434fdd 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -498,8 +498,13 @@ array_agg_transfn(PG_FUNCTION_ARGS) elog(ERROR, "array_agg_transfn called in non-aggregate context"); } - state = PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0); + if (PG_ARGISNULL(0)) + state = initArrayResult(arg1_typeid, aggcontext, false); + else + state = (ArrayBuildState *) PG_GETARG_POINTER(0); + elem = PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1); + state = accumArrayResult(state, elem, PG_ARGISNULL(1), @@ -573,7 +578,22 @@ array_agg_array_transfn(PG_FUNCTION_ARGS) elog(ERROR, "array_agg_array_transfn called in non-aggregate context"); } - state = PG_ARGISNULL(0) ? NULL : (ArrayBuildStateArr *) PG_GETARG_POINTER(0); + + if (PG_ARGISNULL(0)) + { + Oid element_type = get_element_type(arg1_typeid); + + if (!OidIsValid(element_type)) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("data type %s is not an array type", + format_type_be(arg1_typeid)))); + + state = initArrayResultArr(arg1_typeid, element_type, aggcontext, false); + } + else + state = (ArrayBuildStateArr *) PG_GETARG_POINTER(0); + state = accumArrayResultArr(state, PG_GETARG_DATUM(1), PG_ARGISNULL(1), diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index 933c6b0..0302caf 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -4606,6 +4606,7 @@ array_insert_slice(ArrayType *destArray, * * element_type is the array element type (must be a valid array element type) * rcontext is where to keep working state + * subcontext is a flag determining whether to use a separate memory context * * Note: there are two common schemes for using accumArrayResult(). * In the older scheme, you start with a NULL ArrayBuildState pointer, and @@ -4615,24 +4616,36 @@ array_insert_slice(ArrayType *destArray, * once per element. In this scheme you always end with a non-NULL pointer * that you can pass to makeArrayResult; you get an empty array if there * were no elements. This is preferred if an empty array is what you want. + * + * You may choose whether to create a separate memory context for the array + * builder, or whether to allocate it directly within rcontext (along with + * various other pieces). A separate context has the benefit that it may + * be released by deleting the whole context. The main reason why not to + * use private contexts is memory efficiency - the minimum size of a context + * is 8kB (ALLOCSET_DEFAULT_INITSIZE) even if there's a single value in the + * array. When there are many states in parallel (e.g. as in hash aggregate) + * this may result in significant memory bloat (up to causing OOM) and also + * overhead with managing so many memory contexts. */ ArrayBuildState * -initArrayResult(Oid element_type, MemoryContext rcontext) +initArrayResult(Oid element_type, MemoryContext rcontext, bool subcontext) { ArrayBuildState *astate; - MemoryContext arr_context; + MemoryContext arr_context = rcontext; /* by default use the parent ctx */ /* Make a temporary context to hold all the junk */ - arr_context = AllocSetContextCreate(rcontext, - "accumArrayResult", - ALLOCSET_DEFAULT_MINSIZE, - ALLOCSET_DEFAULT_INITSIZE, - ALLOCSET_DEFAULT_MAXSIZE); + if (subcontext) + arr_context = AllocSetContextCreate(rcontext, + "accumArrayResult", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); astate = (ArrayBuildState *) MemoryContextAlloc(arr_context, sizeof(ArrayBuildState)); astate->mcontext = arr_context; - astate->alen = 64; /* arbitrary starting array size */ + astate->private_cxt = subcontext; + astate->alen = 8; /* arbitrary starting array size */ astate->dvalues = (Datum *) MemoryContextAlloc(arr_context, astate->alen * sizeof(Datum)); astate->dnulls = (bool *) @@ -4666,7 +4679,7 @@ accumArrayResult(ArrayBuildState *astate, if (astate == NULL) { /* First time through --- initialize */ - astate = initArrayResult(element_type, rcontext); + astate = initArrayResult(element_type, rcontext, true); } else { @@ -4768,6 +4781,9 @@ makeMdArrayResult(ArrayBuildState *astate, MemoryContextSwitchTo(oldcontext); + /* we can only release the context if it's a private one. */ + Assert(! (release && !astate->private_cxt)); + /* Clean up all the junk */ if (release) MemoryContextDelete(astate->mcontext); @@ -4789,24 +4805,28 @@ makeMdArrayResult(ArrayBuildState *astate, * array_type is the array type (must be a valid varlena array type) * element_type is the type of the array's elements * rcontext is where to keep working state + * subcontext is a flag determining whether to use a separate memory context */ ArrayBuildStateArr * -initArrayResultArr(Oid array_type, Oid element_type, MemoryContext rcontext) +initArrayResultArr(Oid array_type, Oid element_type, MemoryContext rcontext, + bool subcontext) { ArrayBuildStateArr *astate; - MemoryContext arr_context; + MemoryContext arr_context = rcontext; /* by default use the parent ctx */ /* Make a temporary context to hold all the junk */ - arr_context = AllocSetContextCreate(rcontext, - "accumArrayResultArr", - ALLOCSET_DEFAULT_MINSIZE, - ALLOCSET_DEFAULT_INITSIZE, - ALLOCSET_DEFAULT_MAXSIZE); + if (subcontext) + arr_context = AllocSetContextCreate(rcontext, + "accumArrayResultArr", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); /* Note we initialize all fields to zero */ astate = (ArrayBuildStateArr *) MemoryContextAllocZero(arr_context, sizeof(ArrayBuildStateArr)); astate->mcontext = arr_context; + astate->private_cxt = subcontext; /* Save relevant datatype information */ astate->array_type = array_type; @@ -4862,7 +4882,7 @@ accumArrayResultArr(ArrayBuildStateArr *astate, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("data type %s is not an array type", format_type_be(array_type)))); - astate = initArrayResultArr(array_type, element_type, rcontext); + astate = initArrayResultArr(array_type, element_type, rcontext, true); } else { @@ -5044,6 +5064,9 @@ makeArrayResultArr(ArrayBuildStateArr *astate, MemoryContextSwitchTo(oldcontext); + /* we can only release the context if it's a private one. */ + Assert(! (release && !astate->private_cxt)); + /* Clean up all the junk */ if (release) MemoryContextDelete(astate->mcontext); @@ -5062,9 +5085,10 @@ makeArrayResultArr(ArrayBuildStateArr *astate, * * input_type is the input datatype (either element or array type) * rcontext is where to keep working state + * subcontext is a flag determining whether to use a separate memory context */ ArrayBuildStateAny * -initArrayResultAny(Oid input_type, MemoryContext rcontext) +initArrayResultAny(Oid input_type, MemoryContext rcontext, bool subcontext) { ArrayBuildStateAny *astate; Oid element_type = get_element_type(input_type); @@ -5074,7 +5098,7 @@ initArrayResultAny(Oid input_type, MemoryContext rcontext) /* Array case */ ArrayBuildStateArr *arraystate; - arraystate = initArrayResultArr(input_type, element_type, rcontext); + arraystate = initArrayResultArr(input_type, element_type, rcontext, subcontext); astate = (ArrayBuildStateAny *) MemoryContextAlloc(arraystate->mcontext, sizeof(ArrayBuildStateAny)); @@ -5089,7 +5113,7 @@ initArrayResultAny(Oid input_type, MemoryContext rcontext) /* Let's just check that we have a type that can be put into arrays */ Assert(OidIsValid(get_array_type(input_type))); - scalarstate = initArrayResult(input_type, rcontext); + scalarstate = initArrayResult(input_type, rcontext, subcontext); astate = (ArrayBuildStateAny *) MemoryContextAlloc(scalarstate->mcontext, sizeof(ArrayBuildStateAny)); @@ -5115,7 +5139,7 @@ accumArrayResultAny(ArrayBuildStateAny *astate, MemoryContext rcontext) { if (astate == NULL) - astate = initArrayResultAny(input_type, rcontext); + astate = initArrayResultAny(input_type, rcontext, true); if (astate->scalarstate) (void) accumArrayResult(astate->scalarstate, diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index 7d90312..a3fc7b0 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -3930,7 +3930,7 @@ xpath(PG_FUNCTION_ARGS) ArrayType *namespaces = PG_GETARG_ARRAYTYPE_P(2); ArrayBuildState *astate; - astate = initArrayResult(XMLOID, CurrentMemoryContext); + astate = initArrayResult(XMLOID, CurrentMemoryContext, true); xpath_internal(xpath_expr_text, data, namespaces, NULL, astate); PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext)); diff --git a/src/include/utils/array.h b/src/include/utils/array.h index e1d5749..cf0b72c 100644 --- a/src/include/utils/array.h +++ b/src/include/utils/array.h @@ -89,6 +89,7 @@ typedef struct ArrayBuildState int16 typlen; /* needed info about datatype */ bool typbyval; char typalign; + bool private_cxt; /* use private memory context */ } ArrayBuildState; /* @@ -109,6 +110,7 @@ typedef struct ArrayBuildStateArr int lbs[MAXDIM]; Oid array_type; /* data type of the arrays */ Oid element_type; /* data type of the array elements */ + bool private_cxt; /* use private memory context */ } ArrayBuildStateArr; /* @@ -286,7 +288,7 @@ extern void deconstruct_array(ArrayType *array, extern bool array_contains_nulls(ArrayType *array); extern ArrayBuildState *initArrayResult(Oid element_type, - MemoryContext rcontext); + MemoryContext rcontext, bool subcontext); extern ArrayBuildState *accumArrayResult(ArrayBuildState *astate, Datum dvalue, bool disnull, Oid element_type, @@ -297,7 +299,7 @@ extern Datum makeMdArrayResult(ArrayBuildState *astate, int ndims, int *dims, int *lbs, MemoryContext rcontext, bool release); extern ArrayBuildStateArr *initArrayResultArr(Oid array_type, Oid element_type, - MemoryContext rcontext); + MemoryContext rcontext, bool subcontext); extern ArrayBuildStateArr *accumArrayResultArr(ArrayBuildStateArr *astate, Datum dvalue, bool disnull, Oid array_type, @@ -306,7 +308,7 @@ extern Datum makeArrayResultArr(ArrayBuildStateArr *astate, MemoryContext rcontext, bool release); extern ArrayBuildStateAny *initArrayResultAny(Oid input_type, - MemoryContext rcontext); + MemoryContext rcontext, bool subcontext); extern ArrayBuildStateAny *accumArrayResultAny(ArrayBuildStateAny *astate, Datum dvalue, bool disnull, Oid input_type, diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 492c1ef..e3dda5d 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -1218,7 +1218,7 @@ plperl_array_to_datum(SV *src, Oid typid, int32 typmod) errmsg("cannot convert Perl array to non-array type %s", format_type_be(typid)))); - astate = initArrayResult(elemtypid, CurrentMemoryContext); + astate = initArrayResult(elemtypid, CurrentMemoryContext, true); _sv_to_datum_finfo(elemtypid, &finfo, &typioparam);