Re: [HACKERS] Bug in to_timestamp().

On Sun, Jul 8, 2018 at 12:15 AM Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> On Sat, Jul 7, 2018 at 12:31 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)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.

1.
https://www.postgresql.org/message-id/CA%2Bq6zcUS3fQGLoeLcuJLtz-gRD6vHqpn1fBe0cnORdx93QtO4w%40mail.gmail.com

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