Re: [HACKERS] Bug in to_timestamp().

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, amul sul <sulamul(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alex Ignatov <a(dot)ignatov(at)postgrespro(dot)ru>, Bruce Momjian <bruce(at)momjian(dot)us>, amul sul <sul_amul(at)yahoo(dot)co(dot)in>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Bug in to_timestamp().
Date: 2018-09-03 21:30:42
Message-ID: CAPpHfdvP49XweKOnsVf9RPsCjuXkCcm7pxQsRUs9SqxZ6vWydQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi!

Sorry for very long reply.

On Thu, Aug 16, 2018 at 11:44 PM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> If the new behavior is an error I don't really have a problem since the need to fix one's queries will be obvious.
>
> "So length of last group of spaces/separators in the pattern should be
> greater or equal to length of spaces/separators in the input string.
> Other previous groups are ignored in Oracle. And that seems
> ridiculous for me."
>
> What do you believe should (or does) happen? Multiple groups always fail or something else? How does this interplay with the detection of the negative timezone offset? I'm not finding the behavior ridiculous at first blush; not to the extent to avoid emulating it in a function whose purpose is emulation. Being more lenient than Oracle seems undesirable. Regardless of the choice made here it should be memorialized in the regression tests.

The current version of patch doesn't really distinguish spaces and
delimiters in format string in non-FX mode. So, spaces and delimiters
are forming single group. For me Oracle behavior is ridiculous at
least because it doesn't allow cases when input string exactly matches
format string.

This one fails:
SELECT to_timestamp('2018- -01 02', 'YYYY- -MM DD') FROM dual

But both this two are working:
SELECT to_timestamp('2018- -01 02', 'YYYY---MM DD') FROM dual
SELECT to_timestamp('2018- -01 02', 'YYYY MM DD') FROM dual

Regarding TZH, Oracle takes into account total number of characters
between placeholders as we do. So, there is no change in this aspect.

> The couple of regression tests that change do so for the better. It would be illuminating to set this up as two patches though, one introducing all of the new regression tests against the current code and then a second patch with the changed behavior with only the affected tests.

OK, here you go. 0001-to_timestamp-regression-test-v17.patch
introduces changes in regression tests and their output for current
master, while 0002-to_timestamp-format-checking-v17.patch contain
changes to to_timestamp itself.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
0001-to_timestamp-regression-test-v17.patch application/x-patch 6.7 KB
0002-to_timestamp-format-checking-v17.patch application/x-patch 13.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-09-03 21:39:16 pointless check in RelationBuildPartitionDesc
Previous Message David Fetter 2018-09-03 20:48:51 Re: CREATE ROUTINE MAPPING