Re: BUG #15141: Faulty ISO 8601 parsing

On Mon, Apr 2, 2018 at 5:17 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> =?utf-8?q?PG_Bug_reporting_form?= <noreply(at)postgresql(dot)org> writes:
> > The time parsing fails on some valid ISO times, with some locales, e.g.:
>
> > # select to_timestamp('2018-04-03T01:45:00,728456785+0000')::timestamp
> with
> > time zone;
> > ERROR: invalid input syntax for type double precision:
> > "2018-04-03T01:45:00,728456785+0000"
>
> This is confused: the single-argument form of to_timestamp() takes a
> float8 argument, not a timestamp, which is why the error message is
> phrased the way it is.
>
> I think you meant that this fails:
>
> # select '2018-04-03T01:45:00,728456785+0000'::timestamp with time zone;
> ERROR: invalid input syntax for type timestamp with time zone:
> "2018-04-03T01:45:00,728456785+0000"
>
> which it does, but I don't think we should do anything about it.
> There is not and never has been any dependency on LC_TIME properties in
> PG's timestamp I/O. Considering that we also have DateStyle to cope with,
> as well as a lot more flexibility in the input parser than ISO 8601
> contemplates, I think allowing a comma instead of decimal point here
> would probably create more confusion than benefit.
>

​Ideally it would be as simple as:

diff --git a/src/backend/utils/adt/datetime.c
b/src/backend/utils/adt/datetime.c
index 8375b93c39..4a3c7382f2 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -653,8 +653,8 @@ ParseDateTime(const char *timestr, char *workbuf,
size_t buflen,
else
ftype[nf] = DTK_NUMBER;
}
- /* Leading decimal point? Then fractional seconds... */
- else if (*cp == '.')
+ /* Leading decimal point or comma? Then fractional seconds... */
+ else if (*cp == '.' || *cp == ',')
{
APPEND_CHAR(bufp, bufend, *cp++);
while (isdigit((unsigned char) *cp))

But then one needs to contemplate the impact that has on:

​/* ignore other punctuation but use as delimiter */
else if (ispunct((unsigned char) *cp))
{
cp++;
continue;
}

Its not LC_TIME dependent but a bit more complex implementation wise to
detect commas used as separators in custom formats and a comma used as a
fractional separator.

I don't foresee much end-user confusion involved here - especially if we
only allow for fractional seconds at a relatively fixed location.

David J.