From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@postgreSQL.org
Subject: Proposed fix for bug #14826 (the invalid empty array business)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <20324.1506197972.0@sss.pgh.pa.us>
Date: Sat, 23 Sep 2017 16:26:23 -0400
Message-ID: <20570.1506198383@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20324.1506197972.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="handle-empty-arrays-correctly-always.patch";
	charset="us-ascii"
Content-ID: <20324.1506197972.2@sss.pgh.pa.us>
Content-Description: handle-empty-arrays-correctly-always.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/e=
xecExprInterp.c
index bd8a15d..09abd46 100644
*** a/src/backend/executor/execExprInterp.c
--- b/src/backend/executor/execExprInterp.c
*************** ExecEvalArrayExpr(ExprState *state, Expr
*** 2131,2144 ****
  		Datum	   *dvalues =3D op->d.arrayexpr.elemvalues;
  		bool	   *dnulls =3D op->d.arrayexpr.elemnulls;
=20=20
- 		/* Shouldn't happen here, but if length is 0, return empty array */
- 		if (nelems =3D=3D 0)
- 		{
- 			*op->resvalue =3D
- 				PointerGetDatum(construct_empty_array(element_type));
- 			return;
- 		}
-=20
  		/* setup for 1-D array of the given length */
  		ndims =3D 1;
  		dims[0] =3D nelems;
--- 2131,2136 ----
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arr=
ayfuncs.c
index 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 =3D 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] =3D 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)));
=20=20
- 	/* fast track for empty array */
- 	if (ndims =3D=3D 0)
- 		return construct_empty_array(elmtype);
-=20
  	nelems =3D ArrayGetNItems(ndims, dims);
=20=20
  	/* compute required space */
  	nbytes =3D 0;
  	hasnulls =3D false;
--- 3364,3375 ----
  				 errmsg("number of array dimensions (%d) exceeds the maximum allowed =
(%d)",
  						ndims, MAXDIM)));
=20=20
  	nelems =3D ArrayGetNItems(ndims, dims);
=20=20
+ 	/* if ndims <=3D 0 or any dims[i] =3D=3D 0, return empty array */
+ 	if (nelems <=3D 0)
+ 		return construct_empty_array(elmtype);
+=20
  	/* compute required space */
  	nbytes =3D 0;
  	hasnulls =3D false;

------- =_aaaaaaaaaa0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

------- =_aaaaaaaaaa0--