[ cc'ing pghackers in case anyone wants to object ] Joe Conway <[EMAIL PROTECTED]> 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;
---------------------------(end of broadcast)--------------------------- TIP 7: don't forget to increase your free space map settings
