On Thu, Aug 16, 2018 at 3:53 AM, Alexander Korotkov < a.korot...@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.
