Thread: bug in json_to_record with arrays
Tested on 9.4b3, 9.4rc1, 9.5devel
select * from json_to_record('
{"id":1,"val":"josh","valry":["potter","chef","programmer"]}') as r(id
int, val text, valry text[]);
ERROR: missing dimension value
With some experimentation, I can't find any way to convert a JSON array
to an array field using json_to_record or json_to_recordset. I know
this worked back in January, though.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
On 11/26/2014 11:54 AM, Josh Berkus wrote:
> Tested on 9.4b3, 9.4rc1, 9.5devel
>
> select * from json_to_record('
> {"id":1,"val":"josh","valry":["potter","chef","programmer"]}') as r(id
> int, val text, valry text[]);
>
> ERROR: missing dimension value
>
> With some experimentation, I can't find any way to convert a JSON array
> to an array field using json_to_record or json_to_recordset. I know
> this worked back in January, though.
Lemme take that back, it didn't work. Just checked an old devel snapshot.
Looks like this is not intended to work, so the only bug is that we need
a less confusing error message.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 11/26/2014 11:54 AM, Josh Berkus wrote:
>> Tested on 9.4b3, 9.4rc1, 9.5devel
>>
>> select * from json_to_record('
>> {"id":1,"val":"josh","valry":["potter","chef","programmer"]}') as r(id
>> int, val text, valry text[]);
>>
>> ERROR: missing dimension value
>>
>> With some experimentation, I can't find any way to convert a JSON array
>> to an array field using json_to_record or json_to_recordset. I know
>> this worked back in January, though.
> Lemme take that back, it didn't work. Just checked an old devel snapshot.
> Looks like this is not intended to work, so the only bug is that we need
> a less confusing error message.
What's happening is that this string:
["potter","chef","programmer"]
is getting passed to array_in, which of course does not like it because
that's not the I/O syntax for Postgres arrays. Arguably,
populate_record_worker should be smart enough to convert somehow, but
it isn't today. Looks to me like it wouldn't succeed for the comparable
case of converting a sub-object to a Postgres composite type, either.
I'm satisfied with regarding those cases as missing features to be
added later.
As far as your request for a better error message is concerned, I'm a
bit inclined to lay the blame on array_in rather than the JSON code.
Wouldn't it be better if it said
ERROR: invalid input syntax for array: "["potter","chef","programmer"]" DETAIL: Dimension value is missing.
which is comparable to what you'd get out of most other input functions
that were feeling indigestion?
regards, tom lane
On 11/26/2014 03:48 PM, Tom Lane wrote: > Arguably, > populate_record_worker should be smart enough to convert somehow, but > it isn't today. Looks to me like it wouldn't succeed for the comparable > case of converting a sub-object to a Postgres composite type, either. > I'm satisfied with regarding those cases as missing features to be > added later. Right. Before commit a749a23d7af4dba9b3468076ec561d2cbf69af09 it didn't try by default to treat the value as text, but instead errored out if the value was an array or object, with an appropriate message. Now we always try to treat it as text and pass that to the array or composite input functions, who now get the responsibility of telling us what's wrong. It might be possible to process such values recursively, but it would be far from trivial. > > As far as your request for a better error message is concerned, I'm a > bit inclined to lay the blame on array_in rather than the JSON code. > Wouldn't it be better if it said > > ERROR: invalid input syntax for array: "["potter","chef","programmer"]" > DETAIL: Dimension value is missing. > > which is comparable to what you'd get out of most other input functions > that were feeling indigestion? > > +1 cheers andrew
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> As far as your request for a better error message is concerned, I'm a
> bit inclined to lay the blame on array_in rather than the JSON code.
> Wouldn't it be better if it said
>
> ERROR: invalid input syntax for array: "["potter","chef","programmer"]"
> DETAIL: Dimension value is missing.
Sounds pretty reasonable to me, but I would just caution that we should
check if that's considered 'leakproof' or not (or, if it is, if it'd
ever possibly leak data it shouldn't or if it would only ever return
information provided by the user).
Otherwise, someone might be able to convince the planner to push it down
below a security qual and expose data from rows which shouldn't be
visible.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> As far as your request for a better error message is concerned, I'm a
>> bit inclined to lay the blame on array_in rather than the JSON code.
>> Wouldn't it be better if it said
>>
>> ERROR: invalid input syntax for array: "["potter","chef","programmer"]"
>> DETAIL: Dimension value is missing.
> Sounds pretty reasonable to me, but I would just caution that we should
> check if that's considered 'leakproof' or not (or, if it is, if it'd
> ever possibly leak data it shouldn't or if it would only ever return
> information provided by the user).
array_in could only be regarded as leakproof if every element-type input
function it could ever call is also leakproof. Which ain't the case,
so I sure hope it's not marked that way. (Likewise record_in, range_in,
etc.)
regards, tom lane
On 11/26/2014 12:48 PM, Tom Lane wrote: > Wouldn't it be better if it said > > ERROR: invalid input syntax for array: "["potter","chef","programmer"]" > DETAIL: Dimension value is missing. > > which is comparable to what you'd get out of most other input functions > that were feeling indigestion? Yes, it most definitely would be better. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 11/26/2014 12:48 PM, Tom Lane wrote:
>> Wouldn't it be better if it said
>>
>> ERROR: invalid input syntax for array: "["potter","chef","programmer"]"
>> DETAIL: Dimension value is missing.
>>
>> which is comparable to what you'd get out of most other input functions
>> that were feeling indigestion?
> Yes, it most definitely would be better.
Here's a draft patch to improve array_in's error reports. With this
patch, what you'd get for this example is
# select * from json_to_record('
{"id":1,"val":"josh","valry":["potter","chef","programmer"]}') as r(id
int, val text, valry text[]);
ERROR: malformed array literal: "["potter","chef","programmer"]"
DETAIL: "[" must introduce explicitly-specified array dimensions.
Some comments:
* Probably the main objection that could be leveled against this approach
is that it's reasonable to expect that array literal strings could be
pretty long, maybe longer than is reasonable to print all of in a primary
error message. However, the same could be said about record literal
strings; yet I've not heard any complaints about the fact that record_in
just prints the whole input string when it's unhappy. So that's what this
does too.
* The phrasing "malformed array literal" matches some already-existing
error texts, as well as record_in which uses "malformed record literal".
I wonder though if we shouldn't change all of these to "invalid input
syntax for array" (resp. "record"), which seems more like our usual
phraseology in other datatype input functions. OTOH, that would make
the primary error message even longer.
* I changed every one of the ERRCODE_INVALID_TEXT_REPRESENTATION cases
to "malformed array literal" with an errdetail. I did not touch some
existing ereport's with different SQLSTATEs, notably "upper bound cannot
be less than lower bound". Anyone have a different opinion about whether
that case needs to print the string contents?
* Some of the errdetails don't really seem all that helpful (the ones
triggered by the existing regression test cases are examples). Can anyone
think of better wording? Should we just leave those out?
* This would only really address Josh's complaint if we were to back-patch
it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts?
regards, tom lane
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 743351b..933c6b0 100644
*** a/src/backend/utils/adt/arrayfuncs.c
--- b/src/backend/utils/adt/arrayfuncs.c
*************** array_in(PG_FUNCTION_ARGS)
*** 247,257 ****
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
ndim + 1, MAXDIM)));
! for (q = p; isdigit((unsigned char) *q) || (*q == '-') || (*q == '+'); q++);
if (q == p) /* no digits? */
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("missing dimension value")));
if (*q == ':')
{
--- 247,259 ----
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
ndim + 1, MAXDIM)));
! for (q = p; isdigit((unsigned char) *q) || (*q == '-') || (*q == '+'); q++)
! /* skip */ ;
if (q == p) /* no digits? */
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("\"[\" must introduce explicitly-specified array dimensions.")));
if (*q == ':')
{
*************** array_in(PG_FUNCTION_ARGS)
*** 259,269 ****
*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),
! errmsg("missing dimension value")));
}
else
{
--- 261,273 ----
*q = '\0';
lBound[ndim] = atoi(p);
p = q + 1;
! for (q = p; isdigit((unsigned char) *q) || (*q == '-') || (*q == '+'); q++)
! /* skip */ ;
if (q == p) /* no digits? */
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Missing array dimension value.")));
}
else
{
*************** array_in(PG_FUNCTION_ARGS)
*** 273,279 ****
if (*q != ']')
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("missing \"]\" in array dimensions")));
*q = '\0';
ub = atoi(p);
--- 277,285 ----
if (*q != ']')
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Missing \"%s\" after array dimensions.",
! "]")));
*q = '\0';
ub = atoi(p);
*************** array_in(PG_FUNCTION_ARGS)
*** 293,299 ****
if (*p != '{')
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("array value must start with \"{\" or dimension information")));
ndim = ArrayCount(p, dim, typdelim);
for (i = 0; i < ndim; i++)
lBound[i] = 1;
--- 299,306 ----
if (*p != '{')
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Array value must start with \"{\" or dimension information.")));
ndim = ArrayCount(p, dim, typdelim);
for (i = 0; i < ndim; i++)
lBound[i] = 1;
*************** array_in(PG_FUNCTION_ARGS)
*** 307,313 ****
if (strncmp(p, ASSGN, strlen(ASSGN)) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("missing assignment operator")));
p += strlen(ASSGN);
while (array_isspace(*p))
p++;
--- 314,322 ----
if (strncmp(p, ASSGN, strlen(ASSGN)) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Missing \"%s\" after array dimensions.",
! ASSGN)));
p += strlen(ASSGN);
while (array_isspace(*p))
p++;
*************** array_in(PG_FUNCTION_ARGS)
*** 319,336 ****
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")));
}
}
--- 328,348 ----
if (*p != '{')
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Array contents must start with \"{\".")));
ndim_braces = ArrayCount(p, dim_braces, typdelim);
if (ndim_braces != ndim)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Specified array dimensions do not match array contents.")));
for (i = 0; i < ndim; ++i)
{
if (dim[i] != dim_braces[i])
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", string),
! errdetail("Specified array dimensions do not match array contents.")));
}
}
*************** ArrayCount(const char *str, int *dim, ch
*** 460,466 ****
/* Signal a premature end of the string */
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
break;
case '\\':
--- 472,479 ----
/* Signal a premature end of the string */
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected end of input.")));
break;
case '\\':
*************** ArrayCount(const char *str, int *dim, ch
*** 475,481 ****
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
if (parse_state != ARRAY_QUOTED_ELEM_STARTED)
parse_state = ARRAY_ELEM_STARTED;
/* skip the escaped character */
--- 488,496 ----
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected \"%c\" character.",
! '\\')));
if (parse_state != ARRAY_QUOTED_ELEM_STARTED)
parse_state = ARRAY_ELEM_STARTED;
/* skip the escaped character */
*************** ArrayCount(const char *str, int *dim, ch
*** 484,490 ****
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
break;
case '\"':
--- 499,506 ----
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected end of input.")));
break;
case '\"':
*************** ArrayCount(const char *str, int *dim, ch
*** 498,504 ****
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
in_quotes = !in_quotes;
if (in_quotes)
parse_state = ARRAY_QUOTED_ELEM_STARTED;
--- 514,521 ----
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected array element.")));
in_quotes = !in_quotes;
if (in_quotes)
parse_state = ARRAY_QUOTED_ELEM_STARTED;
*************** ArrayCount(const char *str, int *dim, ch
*** 518,524 ****
parse_state != ARRAY_LEVEL_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
parse_state = ARRAY_LEVEL_STARTED;
if (nest_level >= MAXDIM)
ereport(ERROR,
--- 535,543 ----
parse_state != ARRAY_LEVEL_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected \"%c\" character.",
! '{')));
parse_state = ARRAY_LEVEL_STARTED;
if (nest_level >= MAXDIM)
ereport(ERROR,
*************** ArrayCount(const char *str, int *dim, ch
*** 546,566 ****
!(nest_level == 1 && parse_state == ARRAY_LEVEL_STARTED))
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
parse_state = ARRAY_LEVEL_COMPLETED;
if (nest_level == 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
nest_level--;
if (nelems_last[nest_level] != 0 &&
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)
--- 565,589 ----
!(nest_level == 1 && parse_state == ARRAY_LEVEL_STARTED))
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected \"%c\" character.",
! '}')));
parse_state = ARRAY_LEVEL_COMPLETED;
if (nest_level == 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unmatched \"%c\" character.", '}')));
nest_level--;
if (nelems_last[nest_level] != 0 &&
nelems[nest_level] != nelems_last[nest_level])
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Multidimensional arrays must have "
! "sub-arrays with matching "
! "dimensions.")));
nelems_last[nest_level] = nelems[nest_level];
nelems[nest_level] = 1;
if (nest_level == 0)
*************** ArrayCount(const char *str, int *dim, ch
*** 591,597 ****
parse_state != ARRAY_LEVEL_COMPLETED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
if (parse_state == ARRAY_LEVEL_COMPLETED)
parse_state = ARRAY_LEVEL_DELIMITED;
else
--- 614,622 ----
parse_state != ARRAY_LEVEL_COMPLETED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected \"%c\" character.",
! typdelim)));
if (parse_state == ARRAY_LEVEL_COMPLETED)
parse_state = ARRAY_LEVEL_DELIMITED;
else
*************** ArrayCount(const char *str, int *dim, ch
*** 612,618 ****
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
parse_state = ARRAY_ELEM_STARTED;
}
}
--- 637,644 ----
parse_state != ARRAY_ELEM_DELIMITED)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Unexpected array element.")));
parse_state = ARRAY_ELEM_STARTED;
}
}
*************** ArrayCount(const char *str, int *dim, ch
*** 631,637 ****
if (!array_isspace(*ptr++))
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str)));
}
/* special case for an empty array */
--- 657,664 ----
if (!array_isspace(*ptr++))
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg("malformed array literal: \"%s\"", str),
! errdetail("Junk after closing right brace.")));
}
/* special case for an empty array */
*************** ReadArrayStr(char *arrayStr,
*** 718,724 ****
* character.
*
* The error checking in this routine is mostly pro-forma, since we expect
! * that ArrayCount() already validated the string.
*/
srcptr = arrayStr;
while (!eoArray)
--- 745,752 ----
* character.
*
* The error checking in this routine is mostly pro-forma, since we expect
! * that ArrayCount() already validated the string. So we don't bother
! * with errdetail messages.
*/
srcptr = arrayStr;
while (!eoArray)
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index cb606af..d33c9b9 100644
*** a/src/test/regress/expected/arrays.out
--- b/src/test/regress/expected/arrays.out
*************** select '{{1,{2}},{2,3}}'::text[];
*** 1065,1090 ****
--- 1065,1096 ----
ERROR: malformed array literal: "{{1,{2}},{2,3}}"
LINE 1: select '{{1,{2}},{2,3}}'::text[];
^
+ DETAIL: Unexpected "{" character.
select '{{},{}}'::text[];
ERROR: malformed array literal: "{{},{}}"
LINE 1: select '{{},{}}'::text[];
^
+ DETAIL: Unexpected "}" character.
select E'{{1,2},\\{2,3}}'::text[];
ERROR: malformed array literal: "{{1,2},\{2,3}}"
LINE 1: select E'{{1,2},\\{2,3}}'::text[];
^
+ DETAIL: Unexpected "\" character.
select '{{"1 2" x},{3}}'::text[];
ERROR: malformed array literal: "{{"1 2" x},{3}}"
LINE 1: select '{{"1 2" x},{3}}'::text[];
^
+ DETAIL: Unexpected array element.
select '{}}'::text[];
ERROR: malformed array literal: "{}}"
LINE 1: select '{}}'::text[];
^
+ DETAIL: Junk after closing right brace.
select '{ }}'::text[];
ERROR: malformed array literal: "{ }}"
LINE 1: select '{ }}'::text[];
^
+ DETAIL: Junk after closing right brace.
select array[];
ERROR: cannot determine type of empty array
LINE 1: select array[];
On 11/28/2014 12:55 PM, Tom Lane wrote: > * This would only really address Josh's complaint if we were to back-patch > it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts? If we don't fix an error text in an RC, what would we fix in an RC? That's as minor as things get. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 11/28/2014 12:55 PM, Tom Lane wrote:
>> * This would only really address Josh's complaint if we were to back-patch
>> it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts?
> If we don't fix an error text in an RC, what would we fix in an RC?
> That's as minor as things get.
Code-wise, yeah, but it would put some pressure on the translators.
What did you think of the new error texts in themselves?
regards, tom lane
On 12/01/2014 12:28 PM, Josh Berkus wrote: > > On 11/28/2014 12:55 PM, Tom Lane wrote: >> * This would only really address Josh's complaint if we were to back-patch >> it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts? > > If we don't fix an error text in an RC, what would we fix in an RC? > That's as minor as things get. I think the idea is that you only fix *major* things in an RC? That said, I agree that we should fix something so minor. JD > -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc "If we send our children to Caesar for their education, we should not be surprised when they come back as Romans."
On 2014-12-01 15:30:06 -0500, Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: > > On 11/28/2014 12:55 PM, Tom Lane wrote: > >> * This would only really address Josh's complaint if we were to back-patch > >> it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts? > > > If we don't fix an error text in an RC, what would we fix in an RC? > > That's as minor as things get. > > Code-wise, yeah, but it would put some pressure on the translators. I think a untranslated, but on the point, error message is better than a translated confusing one. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 12/01/2014 12:30 PM, Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: >> On 11/28/2014 12:55 PM, Tom Lane wrote: >>> * This would only really address Josh's complaint if we were to back-patch >>> it into 9.4, but I'm hesitant to change error texts post-RC1. Thoughts? > >> If we don't fix an error text in an RC, what would we fix in an RC? >> That's as minor as things get. > > Code-wise, yeah, but it would put some pressure on the translators. > > What did you think of the new error texts in themselves? I would prefer "invalid input syntax" to "malformed array literal", but I'll take anything we can backpatch. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 12/01/2014 12:30 PM, Tom Lane wrote:
>> Code-wise, yeah, but it would put some pressure on the translators.
>>
>> What did you think of the new error texts in themselves?
> I would prefer "invalid input syntax" to "malformed array literal", but
> I'll take anything we can backpatch.
I think if we're changing them at all, it's about the same cost
no matter what we change them to exactly.
I'd be good with going to "invalid input syntax" if we also change
record_in to match. Anyone have a feeling pro or con about that?
regards, tom lane
On Mon, Dec 1, 2014 at 3:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Josh Berkus <josh@agliodbs.com> writes: >> On 12/01/2014 12:30 PM, Tom Lane wrote: >>> Code-wise, yeah, but it would put some pressure on the translators. >>> >>> What did you think of the new error texts in themselves? > >> I would prefer "invalid input syntax" to "malformed array literal", but >> I'll take anything we can backpatch. > > I think if we're changing them at all, it's about the same cost > no matter what we change them to exactly. > > I'd be good with going to "invalid input syntax" if we also change > record_in to match. Anyone have a feeling pro or con about that? I don't know. "malformed array literal" is a mighty big clue that you have a bad "postgres format" array literal and will be well supported by googling -- this problem isn't new. merlin
Merlin Moncure <mmoncure@gmail.com> writes:
> On Mon, Dec 1, 2014 at 3:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd be good with going to "invalid input syntax" if we also change
>> record_in to match. Anyone have a feeling pro or con about that?
> I don't know. "malformed array literal" is a mighty big clue that you
> have a bad "postgres format" array literal and will be well supported
> by googling -- this problem isn't new.
Good point about googling, and after all we are already using "malformed
array literal" for a subset of these cases. Let's stick with that
wording.
regards, tom lane