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

From David G. Johnston
Subject Re: Bug in to_timestamp().
Date
Msg-id CAKFQuwakgjrix8uUvBySE1PR=833iMadTDoDze2rYAxVGmJOGw@mail.gmail.com
Whole thread Raw
In response to Re: Bug in to_timestamp().  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Thu, Jun 23, 2016 at 2:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Jun 23, 2016 at 1:46 PM, David G. Johnston
<david.g.johnston@gmail.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.

pgsql-hackers by date:

Previous
From: Jim Nasby
Date:
Subject: Re: Bug in to_timestamp().
Next
From: Flavius Anton
Date:
Subject: Re: Protocol buffer support for Postgres