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

From Alexander Korotkov
Subject Re: [HACKERS] Bug in to_timestamp().
Date
Msg-id CAPpHfdsZ2wr0qFwXEWSaV03U_EXOpLv03dFUFt-9O-DyDg-w5Q@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().  ("David G. Johnston" <david.g.johnston@gmail.com>)
List pgsql-hackers
On Sun, Jul 8, 2018 at 12:15 AM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
On Sat, Jul 7, 2018 at 12:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > I tool a look at this patch.  It looks good for me.  It applies
> > cleanly on last master, builds without warnings, passes tests.
> > Functionality seems to be well-covered by documentation and regression
> > tests.  So, I'm going to commit it if no objections.
>
> AFAIR, the argument was mainly over whether we agreed with the proposed
> behavioral changes.  It seems a bit premature to me to commit given that
> there's not consensus on that.

Ok.  I've briefly looked to the thread, and I thought there is
consensus already.  Given your concern, I'll investigate this thread
in detail and come up with summary.

So, I've studies this thread in more details.  Let me share my short summary.

This thread was started from complaint about confusing behavior of to_timestamp() function in some cases.  Basically confusing behavior is related to two issues: interpretation of spaces and separators in both input string and format string, tolerance to invalid dates and times (i.e. 2009-06-40 becomes 2009-07-10).  Fix for the second issue was already committed by Tom Lane (d3cd36a1).  So, the improvement under consideration is interpretation of spaces and separators.

to_timestamp()/to_date() functions are not part of SQL standard.  So, unfortunately we can't use standard as a guideline and have to search for other criteria.  On the one hand to_timestamp()/to_date() is here for quite long time and there might be existing applications relying on its behavior.  Changing the behavior of these functions might appear to be a pain for users.  On the other hand these functions were introduced basically for Oracle compatibility.  So it would be nice to keep their behavior as close to Oracle as possible.  On the third hand, if current behavior of these functions is agreed to be confusing, and new behavior is agreed to be less confusing and more close to Oracle, then we can accept it.  If even it would discourage small fraction of users, who are relying on the behavior which we assume to be confusing, way larger fraction of users would be encouraged by this change.

So, as I get from this thread, current patch brings these function very close to Oracle behavior.  The only divergence found yet is related to handling of unmatched tails of input strings (Oracle throws an error while we swallow that silently) [1].  But this issue can be be addressed by a separate patch.

Patch seems to be in a pretty good shape.  So, the question is whether there is a consensus that we want a backward-incompatible behavior change, which this patch does.

My personal opinion is +1 for committing this, because I see that this patch takes a lot of community efforts on discussion, coding, review etc.  All these efforts give a substantial result in a patch, which makes behavior more Oracle-compatible and (IMHO) more clear.

However, in this thread other opinions were expressed.  In particular, David G. Johnston expressed opinion that we shouldn't change behavior of existing functions, alternatively we could introduce new functions with new behavior.  However, I see David doesn't participate this thread for a quite long time.

Thus, in the current situation I can propose following.  Let's establish some period when people can express objections against committing this patch (reasonable short period, say 1 week).  If no objections were expressed then commit.  Otherwise, discuss the objections expressed.


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

pgsql-hackers by date:

Previous
From: Darafei "Komяpa" Praliaskouski
Date:
Subject: Re: JIT breaks PostGIS
Next
From: David Fetter
Date:
Subject: Remove psql's -W option