pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Arthur Zakirov <zaartur@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Allow to_date() and to_timestamp() to accept localized names
Date: Thu, 23 Jan 2020 12:17:22 +0900
Message-ID: <cfa38397-9fe6-70f2-e081-9aa85eb5942e@gmail.com> (raw)
In-Reply-To: <29432.1579731087@sss.pgh.pa.us>
References: <CAC+AXB2JOe4Dw1teZP-N-isNyDdY3F96oGFVYFxhmWtsbgi=ig@mail.gmail.com>
	<20190925195745.GA21894@alvherre.pgsql>
	<CAC+AXB12A7Fr9djoq7f1-WYesnZTRKpGpHM12MRG+M00mewN9g@mail.gmail.com>
	<20200111160602.4wwvyl7lo3oxoewf@development>
	<CAC+AXB1rLC7Gu7-fTQPNwSY9sFC9hkpn8_J3WyGwaKXvNZnoVQ@mail.gmail.com>
	<ba6fa1fd-ad60-d1ea-7226-676790c108ca@gmail.com>
	<CAC+AXB2a6ei0mPFG7h-NaiBVO5=tKT67rONZmrDzoOcAE-B2Sg@mail.gmail.com>
	<31691.1579648824@sss.pgh.pa.us>
	<29432.1579731087@sss.pgh.pa.us>

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

Attachments:

0003-formatting-show-remaining.patch (text/plain, 923B) download 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;

view thread (66+ messages)  latest in thread

Message-ID: <cfa38397-9fe6-70f2-e081-9aa85eb5942e@gmail.com>
Permalink:  ../cfa38397-9fe6-70f2-e081-9aa85eb5942e@gmail.com/
Also on:    postgresql.org/message-id/cfa38397-9fe6-70f2-e081-9aa85eb5942e@gmail.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: zaartur@gmail.com, tgl@sss.pgh.pa.us, juanjo.santamaria@gmail.com, tomas.vondra@2ndquadrant.com, alvherre@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Allow to_date() and to_timestamp() to accept localized names
  In-Reply-To: <cfa38397-9fe6-70f2-e081-9aa85eb5942e@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox