Re: [SQL] ARRAY() returning NULL instead of ARRAY[] resp. {}

From: Joe Conway <mail(at)joeconway(dot)com>
To: "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Markus Bertheau <twanger(at)bluetwanger(dot)de>
Subject: Re: [SQL] ARRAY() returning NULL instead of ARRAY[] resp. {}
Date: 2005-05-31 21:31:16
Message-ID: 429CD7A4.7020702@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches pgsql-sql

Joe Conway wrote:
> OK, looks like I'm outnumbered.
>
> But as far as I know, we have never had a way to produce a
> one-dimensional empty array. Empty arrays thus far have been dimensionless.
>
> Assuming we really want an empty 1D array, I created the attached patch.
> This works fine, but now leaves a few oddities to be dealt with, e.g.:
>
> regression=# select array_dims(array(select 1 where false));
> array_dims
> ------------
> [1:0]
> (1 row)
>
> Any thoughts on how this should be handled for an empty 1D array?

Any thoughts or objections? If not, I'll commit the attached in a day or so.

Thanks,

Joe

> ------------------------------------------------------------------------
>
> Index: src/backend/executor/nodeSubplan.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v
> retrieving revision 1.69
> diff -c -r1.69 nodeSubplan.c
> *** src/backend/executor/nodeSubplan.c 6 May 2005 17:24:54 -0000 1.69
> --- src/backend/executor/nodeSubplan.c 26 May 2005 18:52:16 -0000
> ***************
> *** 215,220 ****
> --- 215,221 ----
> ListCell *pvar;
> ListCell *l;
> ArrayBuildState *astate = NULL;
> + Oid element_type = planstate->ps_ResultTupleSlot->tts_tupleDescriptor->attrs[0]->atttypid;
>
> /*
> * We are probably in a short-lived expression-evaluation context.
> ***************
> *** 259,268 ****
> *
> * For EXPR_SUBLINK we require the subplan to produce no more than one
> * tuple, else an error is raised. For ARRAY_SUBLINK we allow the
> ! * subplan to produce more than one tuple. In either case, if zero
> ! * tuples are produced, we return NULL. Assuming we get a tuple, we
> ! * just use its first column (there can be only one non-junk column in
> ! * this case).
> */
> result = BoolGetDatum(subLinkType == ALL_SUBLINK);
> *isNull = false;
> --- 260,269 ----
> *
> * For EXPR_SUBLINK we require the subplan to produce no more than one
> * tuple, else an error is raised. For ARRAY_SUBLINK we allow the
> ! * subplan to produce more than one tuple. In the former case, if zero
> ! * tuples are produced, we return NULL. In the latter, we return an
> ! * empty array. Assuming we get a tuple, we just use its first column
> ! * (there can be only one non-junk column in this case).
> */
> result = BoolGetDatum(subLinkType == ALL_SUBLINK);
> *isNull = false;
> ***************
> *** 432,458 ****
> }
> }
>
> ! if (!found)
> {
> /*
> * deal with empty subplan result. result/isNull were previously
> ! * initialized correctly for all sublink types except EXPR, ARRAY,
> * and MULTIEXPR; for those, return NULL.
> */
> if (subLinkType == EXPR_SUBLINK ||
> - subLinkType == ARRAY_SUBLINK ||
> subLinkType == MULTIEXPR_SUBLINK)
> {
> result = (Datum) 0;
> *isNull = true;
> }
> }
> - else if (subLinkType == ARRAY_SUBLINK)
> - {
> - Assert(astate != NULL);
> - /* We return the result in the caller's context */
> - result = makeArrayResult(astate, oldcontext);
> - }
>
> MemoryContextSwitchTo(oldcontext);
>
> --- 433,459 ----
> }
> }
>
> ! if (subLinkType == ARRAY_SUBLINK)
> ! {
> ! if (!astate)
> ! astate = initArrayResult(element_type, oldcontext);
> ! /* We return the result in the caller's context */
> ! result = makeArrayResult(astate, oldcontext);
> ! }
> ! else if (!found)
> {
> /*
> * deal with empty subplan result. result/isNull were previously
> ! * initialized correctly for all sublink types except EXPR
> * and MULTIEXPR; for those, return NULL.
> */
> if (subLinkType == EXPR_SUBLINK ||
> subLinkType == MULTIEXPR_SUBLINK)
> {
> result = (Datum) 0;
> *isNull = true;
> }
> }
>
> MemoryContextSwitchTo(oldcontext);
>
> ***************
> *** 925,930 ****
> --- 926,932 ----
> ListCell *l;
> bool found = false;
> ArrayBuildState *astate = NULL;
> + Oid element_type = planstate->ps_ResultTupleSlot->tts_tupleDescriptor->attrs[0]->atttypid;
>
> /*
> * Must switch to child query's per-query memory context.
> ***************
> *** 1010,1016 ****
> }
> }
>
> ! if (!found)
> {
> if (subLinkType == EXISTS_SUBLINK)
> {
> --- 1012,1033 ----
> }
> }
>
> ! if (subLinkType == ARRAY_SUBLINK)
> ! {
> ! /* There can be only one param... */
> ! int paramid = linitial_int(subplan->setParam);
> ! ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
> !
> ! prm->execPlan = NULL;
> !
> ! if (!astate)
> ! astate = initArrayResult(element_type, oldcontext);
> !
> ! /* We build the result in query context so it won't disappear */
> ! prm->value = makeArrayResult(astate, econtext->ecxt_per_query_memory);
> ! prm->isnull = false;
> ! }
> ! else if (!found)
> {
> if (subLinkType == EXISTS_SUBLINK)
> {
> ***************
> *** 1035,1052 ****
> }
> }
> }
> - else if (subLinkType == ARRAY_SUBLINK)
> - {
> - /* There can be only one param... */
> - int paramid = linitial_int(subplan->setParam);
> - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
> -
> - Assert(astate != NULL);
> - prm->execPlan = NULL;
> - /* We build the result in query context so it won't disappear */
> - prm->value = makeArrayResult(astate, econtext->ecxt_per_query_memory);
> - prm->isnull = false;
> - }
>
> MemoryContextSwitchTo(oldcontext);
> }
> --- 1052,1057 ----
> Index: src/backend/utils/adt/arrayfuncs.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/utils/adt/arrayfuncs.c,v
> retrieving revision 1.120
> diff -c -r1.120 arrayfuncs.c
> *** src/backend/utils/adt/arrayfuncs.c 1 May 2005 18:56:18 -0000 1.120
> --- src/backend/utils/adt/arrayfuncs.c 26 May 2005 18:52:16 -0000
> ***************
> *** 3252,3257 ****
> --- 3252,3293 ----
> &my_extra->amstate);
> }
>
> +
> + /*
> + * initArrayResult - initialize an ArrayBuildState for an array result
> + *
> + * rcontext is where to keep working state
> + */
> + ArrayBuildState *
> + initArrayResult(Oid element_type, MemoryContext rcontext)
> + {
> + ArrayBuildState *astate;
> + MemoryContext arr_context,
> + oldcontext;
> +
> + /* Make a temporary context to hold all the junk */
> + arr_context = AllocSetContextCreate(rcontext,
> + "accumArrayResult",
> + ALLOCSET_DEFAULT_MINSIZE,
> + ALLOCSET_DEFAULT_INITSIZE,
> + ALLOCSET_DEFAULT_MAXSIZE);
> + oldcontext = MemoryContextSwitchTo(arr_context);
> + astate = (ArrayBuildState *) palloc(sizeof(ArrayBuildState));
> + astate->mcontext = arr_context;
> + astate->dvalues = (Datum *)
> + palloc(ARRAY_ELEMS_CHUNKSIZE * sizeof(Datum));
> + astate->nelems = 0;
> + astate->element_type = element_type;
> + get_typlenbyvalalign(element_type,
> + &astate->typlen,
> + &astate->typbyval,
> + &astate->typalign);
> +
> + MemoryContextSwitchTo(oldcontext);
> +
> + return astate;
> + }
> +
> /*
> * accumArrayResult - accumulate one (more) Datum for an array result
> *
> ***************
> *** 3264,3293 ****
> Oid element_type,
> MemoryContext rcontext)
> {
> ! MemoryContext arr_context,
> ! oldcontext;
>
> if (astate == NULL)
> {
> /* First time through --- initialize */
> !
> ! /* Make a temporary context to hold all the junk */
> ! arr_context = AllocSetContextCreate(rcontext,
> ! "accumArrayResult",
> ! ALLOCSET_DEFAULT_MINSIZE,
> ! ALLOCSET_DEFAULT_INITSIZE,
> ! ALLOCSET_DEFAULT_MAXSIZE);
> ! oldcontext = MemoryContextSwitchTo(arr_context);
> ! astate = (ArrayBuildState *) palloc(sizeof(ArrayBuildState));
> ! astate->mcontext = arr_context;
> ! astate->dvalues = (Datum *)
> ! palloc(ARRAY_ELEMS_CHUNKSIZE * sizeof(Datum));
> ! astate->nelems = 0;
> ! astate->element_type = element_type;
> ! get_typlenbyvalalign(element_type,
> ! &astate->typlen,
> ! &astate->typbyval,
> ! &astate->typalign);
> }
> else
> {
> --- 3300,3311 ----
> Oid element_type,
> MemoryContext rcontext)
> {
> ! MemoryContext oldcontext;
>
> if (astate == NULL)
> {
> /* First time through --- initialize */
> ! astate = initArrayResult(element_type, rcontext);
> }
> else
> {
> Index: src/include/utils/array.h
> ===================================================================
> RCS file: /cvsroot/pgsql/src/include/utils/array.h,v
> retrieving revision 1.54
> diff -c -r1.54 array.h
> *** src/include/utils/array.h 29 Mar 2005 00:17:18 -0000 1.54
> --- src/include/utils/array.h 26 May 2005 18:52:16 -0000
> ***************
> *** 176,181 ****
> --- 176,182 ----
> Oid elmtype,
> int elmlen, bool elmbyval, char elmalign,
> Datum **elemsp, int *nelemsp);
> + extern ArrayBuildState *initArrayResult(Oid element_type, MemoryContext rcontext);
> extern ArrayBuildState *accumArrayResult(ArrayBuildState *astate,
> Datum dvalue, bool disnull,
> Oid element_type,
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2005-05-31 21:58:03 Re: [SQL] ARRAY() returning NULL instead of ARRAY[] resp. {}
Previous Message Tom Lane 2005-05-31 21:24:12 Re: Deadlock with tsearch2 index ...

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2005-05-31 21:58:03 Re: [SQL] ARRAY() returning NULL instead of ARRAY[] resp. {}
Previous Message Bruno Wolff III 2005-05-31 15:09:43 Re: Backslash handling in strings

Browse pgsql-sql by date

  From Date Subject
Next Message Tom Lane 2005-05-31 21:58:03 Re: [SQL] ARRAY() returning NULL instead of ARRAY[] resp. {}
Previous Message Andrew Hammond 2005-05-31 21:04:27 Re: Sum() rows