Re: [HACKERS] Bug in to_timestamp(). - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: [HACKERS] Bug in to_timestamp().
Date
Msg-id CAKFQuwY5KOaYufbhmyquij2QBF=wZTPaNB_BY_FavMnUm+9RRA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Bug in to_timestamp().  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: [HACKERS] Bug in to_timestamp().
List pgsql-hackers
On Thu, Aug 16, 2018 at 3:53 AM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
Hi, David!

You can notice that:

1) We identified downside of changes in to_timestamp() function and
documented them [1].
2) We found 4 more differences between between patches behavior and
Oracle behavior [2][3].  One of them was assumed to be ridiculous and
wasn't implemented.  So, I think this answers your original concern
that we shouldn't copy Oracle behavior bug to bug.  So, it's depends
on particular case.  For me, if function was introduced for Oracle
compatibility, then it's OK to copy aspects of Oracle behavior that
look reasonable.  But if aspect doesn't look reasonable, then we don't
copy it.

Do you have any feedback on current state of patch?

1. https://www.postgresql.org/message-id/20180723141254.GA10168%40zakirov.localdomain
2. https://www.postgresql.org/message-id/CAPpHfdtqOSniGJRvJ2zaaE8%3DeMB8XDnzvVS-9c3Xufaw%3DiPA%2BQ%40mail.gmail.com
3. https://www.postgresql.org/message-id/CAPpHfdso_Yvbo-EXKD8t3cuAeR7wszPyuWNBdjQLi1NrMt3O5w%40mail.gmail.com

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 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.

David J.

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: C99 compliance for src/port/snprintf.c
Next
From: Tomas Vondra
Date:
Subject: Re: patch to allow disable of WAL recycling