Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 16, 2018 at 3:53 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)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.