Re: Bug in to_timestamp().

On Thu, Jun 23, 2016 at 2:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Jun 23, 2016 at 1:46 PM, David G. Johnston
> <david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> >> Sheesh. Who put you in charge of this? You basically seem like you
> >> are trying to shut up anyone who supports this change, and I don't
> >> think that's right.
> >
> > I'm all for a change in this area - though I'm not impacted enough to
> > actually work on a design proposal.
>
> You weren't for a change in this area a few emails ago; back then, you
> said "I don't see that changing it is a worthwhile endeavor".
>

​I still don't see changing to_timestamp as being the right approach...but
that's just my opinion. I do think that this area "text to timestamp
conversions" could stand to be improved. I may not have been clear on that
distinction.​ But changing the behavior of a function that has been around
since 2000 seems to be just asking for trouble.

> > And I'm not sure how asking for ideas
> > constitutes trying to shut people up. Especially since if no one does
> float
> > a proposal we'll simply have this discussion next year when someone new
> > discovers how badly behaved this function is.
>
> In my opinion, telling people that their emails are not constructive
> and that no change is possible for 9.6 is pretty much the same thing
> as trying to shut people up. And that's what you did.
>

​I guess I should be a bit more careful to make sure that two separate
responses ​on different aspects of a topic cannot be so easily construed as
"this thread is pointless". To be clear I did and still do believe that
getting a change in this area into 10.0 is worthwhile; and that our policy
(and present circumstances) appears to preclude this changing in 9.6. But
as noted below this is just an observation.

> >> My main point is that I'm inclined to deprecate it.
> >>
> >> I can almost guarantee that would make a lot of users very unhappy.
> >> This function is widely used.
> >
> > Tell people not to use. We'd leave it in, unchanged, on backward
> > compatibility grounds. This seems like acceptable behavior for the
> project.
> > Am I mistaken?
>
> Yes. We're not going to deprecate widely-used functionality. We
> might, however, fix it, contrary to your representations upthread.
>

​At this point I feel you are cherry-picking my words to fit your present
feelings. I stand by everything I've written upthread regarding
deprecation and fixing. I'm personally in favor of the former. I'll add
that you are a committer, I am not. The one thing going for change is if
we indeed exactly match the reference behavior and then document it as
being a compatibility function. I hadn't fully pondered this goal and how
its plays into changing 16 year old behavior. Obviously a new name for the
function doesn't cut it in this scenario.

> >> > My second point is if you are going to use this badly designed
> function
> >> > you
> >> > need to protect yourself.
> >>
> >> I agree that anyone using this function should test their format
> >> strings carefully.
> >>
> >> > My understanding is that is not going to change for 9.6.
> >>
> >> That's exactly what is under discussion here.
> >>
> >
> > Ok. I'm having trouble seeing this justified as a bug fix - the docs
> > clearly state our behavior is intentional. Improved behavior, yes, but
> > that's a feature and we're in beta2. Please be specific if you believe
> I've
> > misinterpreted project policies on this matter.
>
> I would not vote to back-patch a change in this area because I think
> that could break SQL code that works today. But I think the current
> behavior is pretty well indefensible. It's neither intuitive nor
> compatible with Oracle. And there is plenty of existing precedent for
> making this sort of change during the beta period. We regard it as a
> bug fix which is too dangerous to back-patch; therefore, it is neither
> subject to feature freeze nor does it go into existing stable
> releases. Whether to treat this particular issue in that particular
> way is something that needs to be hammered out in discussion. Tom
> raises the valid point that we need to make sure that we've thoroughly
> studied this issue and fixed all of the related cases before
> committing anything -- we don't want to change the behavior in
> 9.6beta, release 9.6, and then have to change it again for 9.7. But
> there is certainly no project policy which says we can't change this
> now for 9.6 if we decide that (1) the current behavior is wrong and
> (2) we are quite sure we know how to fix it.
>
>
Thank You.

I still conclude that given the general lack of complaints, the fact we are
at beta2, the age of the feature and nature of the "bug", and the
non-existence of a working patch even for HEAD, that 9.6 is not going to
see this behavior changed and you will be loath to back-patch a fix once
9.6 becomes stable. I'll admit the possibility does exist but am not all
that keen on couching every statement of this form I make. If you find my
interpretation to be overly conservative then voice yours. I've been doing
this a while without any complaint so I'm apparently right considerably
more often than I am wrong.

You are welcome to say that you'd consider a patch for 9.6 if its received
by the end of beta2 - or that you are working on one yourself and hope to
have it possibly included in time for beta3. I'll go buy and then eat a
hat if this happens and we have backward incompatibility note for
to_timestamp in the 9.6 docs.

David J.