Re: [BUGS] casting strings to multidimensional arrays yields strange - Mailing list pgsql-patches

Tom Lane wrote:
> [ cc'ing pghackers in case anyone wants to object ]
> Joe Conway <mail@joeconway.com> writes:
>>I think that even once we support NULL array elements, they should be
>>explicitly requested -- i.e. throwing an error on non-rectangular input
>>is still the right thing to do. I haven't suggested that in the past
>>because of the backward-compatibility issue, but maybe now is the time
>>to bite the bullet.
>
> Okay with me.  Anyone on pghackers not happy?
>
>>If you think this qualifies as a bug fix for 7.5, I can take a look at
>>it next week.
>
> Yeah, we can call it a bug fix.

The attached addresses the above issue as well as the ones mentioned in
my RFC post from yesterday found here:
http://archives.postgresql.org/pgsql-hackers/2004-08/msg00126.php

So whereas you used to get this:
test=# select '{{{1,2},{4,5,6}},{7,8}}'::int[];
  int4
------
  {}
(1 row)

you now get this:
test=# select '{{{1,2},{4,5,6}},{7,8}}'::int[];
ERROR:  multidimensional arrays must have array expressions with
matching dimensions

Negative lower bound indicies now work also, and array_out will always
output explicit dimensions for an array with any dimension having a
lower bound of other than one. This allows the dimensions to be
preserved across pg_dump/reload cycles:

CREATE TABLE foo (
     f1 integer[]
);
COPY foo (f1) FROM stdin;
[1:1][3:4][-2:0]={{{1,2,3},{4,5,6}}}
[-42:-41][12:14]={{1,2,3},{4,5,6}}
\.

test=# select f1, array_lower(f1, 1) as lb1, array_lower(f1, 2) as lb2,
array_lower(f1, 3) as lb3 from foo;
                   f1                  | lb1 | lb2 | lb3
--------------------------------------+-----+-----+-----
  [1:1][3:4][-2:0]={{{1,2,3},{4,5,6}}} |   1 |   3 |  -2
  [-42:-41][12:14]={{1,2,3},{4,5,6}}   | -42 |  12 |
(2 rows)

If there are no objections, I'll adjust the appropriate regression
script/expected files and commit tonight. And of course I'll follow up
with documentation updates too.

Any thoughts on whether or not to apply this to 7.4?

Thanks,

Joe
Index: src/backend/utils/adt/arrayfuncs.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/adt/arrayfuncs.c,v
retrieving revision 1.105
diff -c -r1.105 arrayfuncs.c
*** src/backend/utils/adt/arrayfuncs.c    16 Jun 2004 01:26:47 -0000    1.105
--- src/backend/utils/adt/arrayfuncs.c    4 Aug 2004 15:37:34 -0000
***************
*** 217,223 ****
                       errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
                              ndim, MAXDIM)));

!         for (q = p; isdigit((unsigned char) *q); q++);
          if (q == p)                /* no digits? */
              ereport(ERROR,
                      (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
--- 217,223 ----
                       errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
                              ndim, MAXDIM)));

!         for (q = p; isdigit((unsigned char) *q) || (*q == '-') || (*q == '+'); q++);
          if (q == p)                /* no digits? */
              ereport(ERROR,
                      (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
***************
*** 229,235 ****
              *q = '\0';
              lBound[ndim] = atoi(p);
              p = q + 1;
!             for (q = p; isdigit((unsigned char) *q); q++);
              if (q == p)            /* no digits? */
                  ereport(ERROR,
                          (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
--- 229,235 ----
              *q = '\0';
              lBound[ndim] = atoi(p);
              p = q + 1;
!             for (q = p; isdigit((unsigned char) *q) || (*q == '-') || (*q == '+'); q++);
              if (q == p)            /* no digits? */
                  ereport(ERROR,
                          (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
***************
*** 270,275 ****
--- 270,278 ----
      }
      else
      {
+         int            ndim_braces,
+                     dim_braces[MAXDIM];
+
          /* If array dimensions are given, expect '=' operator */
          if (strncmp(p, ASSGN, strlen(ASSGN)) != 0)
              ereport(ERROR,
***************
*** 278,283 ****
--- 281,307 ----
          p += strlen(ASSGN);
          while (isspace((unsigned char) *p))
              p++;
+
+         /*
+          * intuit dimensions from brace structure -- it better match what
+          * we were given
+          */
+         if (*p != '{')
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                      errmsg("array value must start with \"{\" or dimension information")));
+         ndim_braces = ArrayCount(p, dim_braces, typdelim);
+         if (ndim_braces != ndim)
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                      errmsg("array dimensions incompatible with array literal")));
+         for (i = 0; i < ndim; ++i)
+         {
+             if (dim[i] != dim_braces[i])
+                 ereport(ERROR,
+                         (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                         errmsg("array dimensions incompatible with array literal")));
+         }
      }

  #ifdef ARRAYDEBUG
***************
*** 303,309 ****
          ereport(ERROR,
                  (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                   errmsg("missing left brace")));
-
      dataPtr = ReadArrayStr(p, nitems, ndim, dim, &my_extra->proc, typioparam,
                             typmod, typdelim, typlen, typbyval, typalign,
                             &nbytes);
--- 327,332 ----
***************
*** 334,346 ****
      int            nest_level = 0,
                  i;
      int            ndim = 1,
!                 temp[MAXDIM];
      bool        scanning_string = false;
      bool        eoArray = false;
      char       *ptr;

      for (i = 0; i < MAXDIM; ++i)
          temp[i] = dim[i] = 0;

      if (strncmp(str, "{}", 2) == 0)
          return 0;
--- 357,374 ----
      int            nest_level = 0,
                  i;
      int            ndim = 1,
!                 temp[MAXDIM],
!                 nelems[MAXDIM],
!                 nelems_last[MAXDIM];
      bool        scanning_string = false;
      bool        eoArray = false;
      char       *ptr;

      for (i = 0; i < MAXDIM; ++i)
+     {
          temp[i] = dim[i] = 0;
+         nelems_last[i] = nelems[i] = 1;
+     }

      if (strncmp(str, "{}", 2) == 0)
          return 0;
***************
*** 394,399 ****
--- 422,437 ----
                              (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                               errmsg("malformed array literal: \"%s\"", str)));
                          nest_level--;
+
+                         if ((nelems_last[nest_level] != 1) &&
+                             (nelems[nest_level] != nelems_last[nest_level]))
+                             ereport(ERROR,
+                                 (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                                  errmsg("multidimensional arrays must have "
+                                         "array expressions with matching "
+                                         "dimensions")));
+                         nelems_last[nest_level] = nelems[nest_level];
+                         nelems[nest_level] = 1;
                          if (nest_level == 0)
                              eoArray = itemdone = true;
                          else
***************
*** 408,414 ****
--- 446,455 ----
                      break;
                  default:
                      if (*ptr == typdelim && !scanning_string)
+                     {
                          itemdone = true;
+                         nelems[nest_level - 1]++;
+                     }
                      break;
              }
              if (!itemdone)
***************
*** 684,699 ****
      char       *p,
                 *tmp,
                 *retval,
!               **values;
!     bool       *needquotes;
      int            nitems,
                  overall_length,
                  i,
                  j,
                  k,
                  indx[MAXDIM];
      int            ndim,
!                *dim;
      ArrayMetaState *my_extra;

      element_type = ARR_ELEMTYPE(v);
--- 725,748 ----
      char       *p,
                 *tmp,
                 *retval,
!               **values,
!                 /*
!                  * each dim: (sign + 10 digits) * 2, semicolon, 2 brackets
!                  * assignment operator + terminator
!                  */
!                 dims_str[(MAXDIM * 25) + 2];
!     bool       *needquotes,
!                 needdims = false;
      int            nitems,
                  overall_length,
+                 dims_str_length = 0,
                  i,
                  j,
                  k,
                  indx[MAXDIM];
      int            ndim,
!                *dim,
!                *lb;
      ArrayMetaState *my_extra;

      element_type = ARR_ELEMTYPE(v);
***************
*** 734,739 ****
--- 783,789 ----

      ndim = ARR_NDIM(v);
      dim = ARR_DIMS(v);
+     lb = ARR_LBOUND(v);
      nitems = ArrayGetNItems(ndim, dim);

      if (nitems == 0)
***************
*** 742,747 ****
--- 792,806 ----
          PG_RETURN_CSTRING(retval);
      }

+     for (i = 0; i < ndim; i++)
+     {
+         if (lb[i] != 1)
+         {
+             needdims = true;
+             break;
+         }
+     }
+
      /*
       * Convert all values to string form, count total space needed
       * (including any overhead such as escaping backslashes), and detect
***************
*** 798,809 ****
       */
      for (i = j = 0, k = 1; i < ndim; k *= dim[i++], j += k);

!     retval = (char *) palloc(overall_length + 2 * j);
      p = retval;

  #define APPENDSTR(str)    (strcpy(p, (str)), p += strlen(p))
  #define APPENDCHAR(ch)    (*p++ = (ch), *p = '\0')

      APPENDCHAR('{');
      for (i = 0; i < ndim; indx[i++] = 0);
      j = 0;
--- 857,896 ----
       */
      for (i = j = 0, k = 1; i < ndim; k *= dim[i++], j += k);

!     if (needdims)
!     {
!         char   *ptr = dims_str;
!
!         for (i = 0; i < ndim; i++)
!         {
!             char    buf_lb[12];    /* sign, 10 digits, '\0' */
!             char    buf_ub[12];    /* sign, 10 digits, '\0' */
!
!             *ptr++ = '[';
!
!             sprintf(buf_lb, "%d", lb[i]);
!             strcpy(ptr, buf_lb), ptr += strlen(buf_lb);
!
!             *ptr++ = ':';
!
!             sprintf(buf_ub, "%d", lb[i] + dim[i] - 1);
!             strcpy(ptr, buf_ub), ptr += strlen(buf_ub);
!
!             *ptr++ = ']';
!         }
!         *ptr++ = *ASSGN;
!         *ptr = '\0';
!         dims_str_length = strlen(dims_str);
!     }
!
!     retval = (char *) palloc(dims_str_length + overall_length + 2 * j);
      p = retval;

  #define APPENDSTR(str)    (strcpy(p, (str)), p += strlen(p))
  #define APPENDCHAR(ch)    (*p++ = (ch), *p = '\0')

+     if (needdims)
+         APPENDSTR(dims_str);
      APPENDCHAR('{');
      for (i = 0; i < ndim; indx[i++] = 0);
      j = 0;