On 2020/01/23 7:11, Tom Lane wrote:
Closer examination shows that the "max" argument is pretty bogus as
well. It doesn't do anything except confuse the reader, because there
are no cases where the value passed is less than the maximum array entry
length, so it never acts to change seq_search's behavior. So we should
just drop that behavior from seq_search, too, and redefine "max" as
having no purpose except to specify how much of the string to show in
error messages. There's still a question of what that should be for
non-English cases, but at least we now have a clear idea of what we
need the value to do.
Shouldn't we just show all remaining string instead of truncating it?
For example I get the following output:
=# select to_timestamp('3 ЯНВАРЯ 1999', 'DD MONTH YYYY');
ERROR: invalid value "ЯНВА" for "MONTH"
DETAIL: The given value did not match any of the allowed values for
this field.
This behavior is reproduced without the patch though (on Postgres 12).
And the English case might confuse too I think:
=# select to_date('3 JANUZRY 1999', 'DD MONTH YYYY');
ERROR: invalid value "JANUZRY 1" for "MONTH"
DETAIL: The given value did not match any of the allowed values for
this field.
Users might think what means "1" in "JANUZRY 1" string.
I attached the draft patch, which shows all remaining string. So the
query above will show the following:
=# select to_timestamp('3 ЯНВАРЯ 1999', 'DD MONTH YYYY');
ERROR: invalid value "ЯНВАРЯ 1999" for "MONTH"
DETAIL: The remaining value did not match any of the allowed values for
this field.
The 0001 patch attached cleans up all the above complaints. I felt
that given the business about scribbling on strings we shouldn't,
it would also be wise to const-ify the string pointers if possible.
That seems not too painful, per 0002 attached.
+1 on the patch.
--
Arthur
diff --git a/src/backend/utils/adt/formatting.c
b/src/backend/utils/adt/formatting.c
index ef21b7e426..63e773c4c1 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -2539,17 +2539,11 @@ from_char_seq_search(int *dest, const char **src, const
char *const *array,
if (len <= 0)
{
- char copy[DCH_MAX_ITEM_SIZ + 1];
-
- /* Use multibyte-aware truncation to avoid generating a bogus
string */
- max = pg_mbcliplen(*src, strlen(*src), max);
- strlcpy(copy, *src, max + 1);
-
RETURN_ERROR(ereport(ERROR,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid value
\"%s\" for \"%s\"",
- copy,
node->key->name),
- errdetail("The given
value did not match any of "
+ *src,
node->key->name),
+ errdetail("The
remaining value did not match any of "
"the allowed values for this field."))));
}
*src += len;
