From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@postgreSQL.org
Subject: Proposed fix for bug #14826 (the invalid empty array business)
Date: Sat, 23 Sep 2017 16:26:23 -0400
Message-ID: <20570.1506198383@sss.pgh.pa.us> (raw)
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
I poked around among the callers of construct_[md_]array() and found at
least two more besides ts_lexize() that could build zero-element arrays;
one of very ancient standing is in the pg_prepared_statements view.
So I think we've got a clear hazard here that justifies changing the
behavior of the low-level array functions, rather than expecting every
call site to be on its guard about empty arrays. Accordingly, I propose
the attached patch which makes construct_md_array responsible for getting
this right.
In principle we could now remove code from the places that do take care to
call construct_empty_array instead. But I found only one place where it
really seemed worth removing anything, in ExecEvalArrayExpr.
I'm a little bit scared about back-patching this, as it seems at least
possible that some external code could be broken by the change in what
construct_[md_]array could hand back. Given that some of these bugs
have been in place for ~ten years and nobody noticed till now, seems
like it might be good enough to fix them in HEAD only.
Comments?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:
[text/x-diff] handle-empty-arrays-correctly-always.patch (2.5K, ../20570.1506198383@sss.pgh.pa.us/2-handle-empty-arrays-correctly-always.patch)
download | inline diff:diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.cindex bd8a15d..09abd46 100644
*** a/src/backend/executor/execExprInterp.c
--- b/src/backend/executor/execExprInterp.c
*************** ExecEvalArrayExpr(ExprState *state, Expr
*** 2131,2144 ****
Datum *dvalues = op->d.arrayexpr.elemvalues;
bool *dnulls = op->d.arrayexpr.elemnulls;
- /* Shouldn't happen here, but if length is 0, return empty array */- if (nelems == 0)- {- *op->resvalue =- PointerGetDatum(construct_empty_array(element_type));- return;- }-
/* setup for 1-D array of the given length */
ndims = 1;
dims[0] = nelems;
--- 2131,2136 ----diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.cindex e4101c9..d1f2fe7 100644
*** a/src/backend/utils/adt/arrayfuncs.c
--- b/src/backend/utils/adt/arrayfuncs.c
*************** array_map(FunctionCallInfo fcinfo, Oid r
*** 3297,3302 ****
--- 3297,3303 ----
*
* A palloc'd 1-D array object is constructed and returned. Note that
* elem values will be copied into the object even if pass-by-ref type.
+ * Also note the result will be 0-D not 1-D if nelems = 0.
*
* NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
* from the system catalogs, given the elmtype. However, the caller is
*************** construct_array(Datum *elems, int nelems
*** 3331,3336 ****
--- 3332,3338 ----
*
* A palloc'd ndims-D array object is constructed and returned. Note that
* elem values will be copied into the object even if pass-by-ref type.
+ * Also note the result will be 0-D not ndims-D if any dims[i] = 0.
*
* NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
* from the system catalogs, given the elmtype. However, the caller is
*************** construct_md_array(Datum *elems,
*** 3362,3373 ****
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
ndims, MAXDIM)));
- /* fast track for empty array */- if (ndims == 0)- return construct_empty_array(elmtype);-
nelems = ArrayGetNItems(ndims, dims);
/* compute required space */
nbytes = 0;
hasnulls = false;
--- 3364,3375 ----
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
ndims, MAXDIM)));
nelems = ArrayGetNItems(ndims, dims);
+ /* if ndims <= 0 or any dims[i] == 0, return empty array */+ if (nelems <= 0)+ return construct_empty_array(elmtype);+
/* compute required space */
nbytes = 0;
hasnulls = false;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us
Subject: Re: Proposed fix for bug #14826 (the invalid empty array business)
In-Reply-To: <20570.1506198383@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox