[HACKERS] Proposed fix for bug #14826 (the invalid empty array business)

Поиск
Список
Период
Сортировка
От Tom Lane
Тема [HACKERS] Proposed fix for bug #14826 (the invalid empty array business)
Дата
Msg-id 20570.1506198383@sss.pgh.pa.us
обсуждение исходный текст
Список pgsql-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

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.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 = 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.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 = 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;

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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?
Следующее
От: Andrew Dunstan
Дата:
Сообщение: Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?