Thread: Bad canonicalization for dateranges with 'infinity' bounds
https://www.postgresql.org/docs/current/rangetypes.html#RANGETYPES-INFINITE has: Also, some element types have a notion of “infinity”, but that is just another value so far as the range type mechanisms are concerned. For example, in timestamp ranges, [today,] means the same thing as [today,). But [today,infinity] means something different from [today,infinity) — the latter excludes the special timestamp value infinity. This does not work as expected for ranges with discrete base types, notably daterange: test=> SELECT '[2000-01-01,infinity]'::daterange; daterange ----------------------- [2000-01-01,infinity) (1 row) test=> SELECT '(-infinity,2000-01-01)'::daterange; daterange ------------------------ [-infinity,2000-01-01) (1 row) This is because "daterange_canonical" makes no difference for 'infinity', and adding one to infinity does not change the value. I propose the attached patch which fixes the problem. Yours, Laurenz Albe
I wrote: > I propose the attached patch which fixes the problem. I forgot to attach the patch. Here it is. Yours, Laurenz Albe
Attachment
On Fri, May 3, 2019 at 12:49 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote: > > I propose the attached patch which fixes the problem. Hi Laurenz, I agree that the patch makes the code match the documentation. The documented behaviour seems to make more sense than the code, since unpatched master gives this nonsense result when it flips the inclusive flag but doesn't adjust the value (because it can't): postgres=# select '(-infinity,infinity]'::daterange @> 'infinity'::date; ?column? ---------- f (1 row) - if (!upper.infinite && upper.inclusive) + if (!(upper.infinite || DATE_NOT_FINITE(upper.val)) && upper.inclusive) Even though !(X || Y) is equivalent to !X && !Y, by my reading of range_in(), lower.value can be uninitialised when lower.infinite is true, and it's also a bit hard to read IMHO, so I'd probably write that as !upper.infinite && !DATE_NOT_FINITE(upper.val) && upper.inclusive. I don't think it can affect the result but it might upset Valgrind or similar. -- Thomas Munro https://enterprisedb.com
On Sun, Jul 14, 2019 at 12:44 AM Thomas Munro <thomas.munro@gmail.com> wrote: > Even though !(X || Y) is equivalent to !X && !Y, by my reading of > range_in(), lower.value can be uninitialised when lower.infinite is > true, and it's also a bit hard to read IMHO, so I'd probably write > that as !upper.infinite && !DATE_NOT_FINITE(upper.val) && > upper.inclusive. I don't think it can affect the result but it might > upset Valgrind or similar. I take back the bit about reading an uninitialised value (X || Y doesn't access Y if X is true... duh), but I still think the other way of putting it is a bit easier to read. YMMV. Generally, +1 for this patch. I'll wait a couple of days for more feedback to appear. -- Thomas Munro https://enterprisedb.com
On Sun, 2019-07-14 at 15:27 +1200, Thomas Munro wrote:
> I take back the bit about reading an uninitialised value (X || Y
> doesn't access Y if X is true... duh), but I still think the other
> way
> of putting it is a bit easier to read. YMMV.
>
> Generally, +1 for this patch. I'll wait a couple of days for more
> feedback to appear.
I went ahead and committed this using Thomas's suggestion to remove the
parentheses.
Thanks!
Regards,
Jeff Davis
Jeff Davis <pgsql@j-davis.com> writes:
> I went ahead and committed this using Thomas's suggestion to remove the
> parentheses.
The commit message claims this was back-patched, but I see no back-patch?
(The commit message doesn't seem to have made it to the pgsql-committers
list either, but that's probably an independent issue.)
regards, tom lane
On Thu, 2019-07-18 at 13:56 -0700, Jeff Davis wrote: > I went ahead and committed this using Thomas's suggestion to remove the > parentheses. Thanks for the review and the commit! Yours, Laurenz Albe
On Thu, 2019-07-18 at 17:36 -0400, Tom Lane wrote:
> The commit message claims this was back-patched, but I see no back-
> patch?
Sorry, I noticed an issue after pushing: we were passing a datum
directly to DATE_NOT_FINITE, when we should have called
DatumGetDateADT() first. I ran through the tests again and now pushed
to all branches.
> (The commit message doesn't seem to have made it to the pgsql-
> committers
> list either, but that's probably an independent issue.)
I was curious about that as well.
Regards,
Jeff Davis
On Thu, Jul 18, 2019 at 05:36:35PM -0400, Tom Lane wrote: > Jeff Davis <pgsql@j-davis.com> writes: > > I went ahead and committed this using Thomas's suggestion to remove the > > parentheses. > > The commit message claims this was back-patched, but I see no back-patch? > > (The commit message doesn't seem to have made it to the pgsql-committers > list either, but that's probably an independent issue.) REL_12_STABLE has been missed in the set of branches patched. Could you fix that as well (including the extra fix b0a7e0f0)? -- Michael
Attachment
Greetings, * Jeff Davis (pgsql@j-davis.com) wrote: > On Thu, 2019-07-18 at 17:36 -0400, Tom Lane wrote: > > (The commit message doesn't seem to have made it to the pgsql- > > committers > > list either, but that's probably an independent issue.) > > I was curious about that as well. The whitelists we put in place expire after a certain period of time (iirc, it's 1 year currently) and then your posts end up getting moderated. If you register that address as an alternate for you, you should be able to post with it without needing to be on a whitelist. Thanks, Stephen