Re: Have I found an interval arithmetic bug? - Mailing list pgsql-hackers

From Bryn Llewellyn
Subject Re: Have I found an interval arithmetic bug?
Date
Msg-id CEA45CB6-7C0B-4960-9913-BD009DD04533@yugabyte.com
Whole thread Raw
In response to Re: Have I found an interval arithmetic bug?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Have I found an interval arithmetic bug?  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 21-Jul-2021, at 02:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
Hmm, looking at this whole thread, I have to say that I prefer the old
behaviour of spilling down to lower units.

For example, with this patch:

SELECT '0.5 weeks'::interval;
interval
----------
4 days

which I don't think is really an improvement. My expectation is that
half a week is 3.5 days, and I prefer what it used to return, namely
'3 days 12:00:00'.

Yeah, that is clearly a significant dis-improvement.

In general, considering that (most of?) the existing behavior has stood
for decades, I think we need to tread VERY carefully about changing it.
I don't want to see this patch changing any case that is not indisputably
broken.

It was me that started the enormous thread with the title “Have I found an interval arithmetic bug?” on 01-Apr-2021. I presented this testcase:

select interval '-1.7 years';                          -- -1 years -8 mons

select interval '29.4 months';                         --  2 years  5 mons 12 days

select interval '-1.7 years 29.4 months';              --           8 mons 12 days << wrong
select interval '29.4 months -1.7 years';              --           9 mons 12 days

select interval '-1.7 years' + interval '29.4 months'; --           9 mons 12 days
select interval '29.4 months' + interval '-1.7 years'; --           9 mons 12 days

The consensus was that the outcome that I flagged with “wrong” does indeed have that status. After all, it’s hard to see how anybody could intend this rule (that anyway holds in only some cases):

-a + b <> b - a

It seems odd that there’s been no recent reference to my testcase and how it behaves in the environment of Bruce’s patch.

I don’t recall the history of the thread. But Bruce took on the task of fixing this narrow issue. Anyway, somehow, the whole question of “spill down” came up for discussion. The rules aren’t documented and I’ve been unable to find any reference even to the phenomenon. I have managed to implement a model, in PL/pgSQL, that gets the same results as the native implementation in every one of many tests that I’ve done. I appreciate that this doesn’t prove that my model is correct. But it would seem that it must be on the right track. The rules that my PL/pgSQL uses are self-evidently whimsical—but they were needed precisely to get the same outcomes as the native implementation. There was some discussion of all this somewhere in this thread.

If memory serves, it was Tom who suggested changing the spill-down rules. This was possibly meant entirely rhetorically. But it seems that Bruce did set about implementing a change here. (I was unable to find a clear prose functional spec for the new behavior. Probably I didn’t know where to look.

There’s no doubt that a change in these rules would change the behavior of extant code. But then, in a purist sense, this is the case with any bug fix.

I’m simply waiting on a final ruling and final outcome.

Meanwhile, I’ve worked out a way to tame all this business (by using domain types and associated functionality) so that application code can deal confidently with only pure months, pure days, and pure seconds interval values (thinking of the internal [mm, dd, ss] representation). The scheme ensures that spill-down never occurs by rounding the years or the months field to integral values. If you want to add a “mixed” interval to a timestamp, then you simply add the different kinds of interval in the one expression. And you use parentheses to assert, visibly, the priority rule that you intend.

Because this is ordinary application code, there are no compatibility issues for me. My approach won’t see a change in behavior no matter what is decided about the present patch.

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Case expression pushdown
Next
From: vignesh C
Date:
Subject: Re: Printing backtrace of postgres processes