Re: [HACKERS] Bug in to_timestamp().

Hi!

Sorry for very long reply.

On Thu, Aug 16, 2018 at 11:44 PM David G. Johnston
<david.g.johnston@gmail.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
withthe detection of the negative timezone offset?  I'm not finding the behavior ridiculous at first blush; not to the
extentto 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
patchesthough, one introducing all of the new regression tests against the current code and then a second patch with
thechanged 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