pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedBad canonicalization for dateranges with 'infinity' bounds
10+ messages / 6 participants
[nested] [flat]
* Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-05-02 12:40 Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 10+ messages in thread
From: Laurenz Albe @ 2019-05-02 12:40 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-05-02 12:49 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 10+ messages in thread
From: Laurenz Albe @ 2019-05-02 12:49 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
I wrote:
> I propose the attached patch which fixes the problem.
I forgot to attach the patch. Here it is.
Yours,
Laurenz Albe
Attachments:
[text/x-patch] 0001-Don-t-canonicalize-dateranges-with-infinity-bounds.patch (3.4K, ../../539fbfe416eb3d7ebf72268eb839163a68aed9ef.camel@cybertec.at/2-0001-Don-t-canonicalize-dateranges-with-infinity-bounds.patch)
download | inline diff:
From 6bbad0acf3baae3a08d1f911b7017642c8a8afe9 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Thu, 2 May 2019 14:32:27 +0200
Subject: [PATCH] Don't canonicalize dateranges with 'infinity' bounds
Since adding one to infinity doesn't change the value, canonicalization
of dateranges with an explicit '-infinity' as exclusive lower bound
would make it an inclusive bound, while dateranges with 'infinity' as
inclusive upper bound would turn it into an exclusive bound.
---
src/backend/utils/adt/rangetypes.c | 4 ++--
src/test/regress/expected/rangetypes.out | 24 ++++++++++++++++++++++++
src/test/regress/sql/rangetypes.sql | 4 ++++
3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index 72c450c70e..a98f17bb66 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -1431,13 +1431,13 @@ daterange_canonical(PG_FUNCTION_ARGS)
if (empty)
PG_RETURN_RANGE_P(r);
- if (!lower.infinite && !lower.inclusive)
+ if (!(lower.infinite || DATE_NOT_FINITE(lower.val)) && !lower.inclusive)
{
lower.val = DirectFunctionCall2(date_pli, lower.val, Int32GetDatum(1));
lower.inclusive = true;
}
- if (!upper.infinite && upper.inclusive)
+ if (!(upper.infinite || DATE_NOT_FINITE(upper.val)) && upper.inclusive)
{
upper.val = DirectFunctionCall2(date_pli, upper.val, Int32GetDatum(1));
upper.inclusive = false;
diff --git a/src/test/regress/expected/rangetypes.out b/src/test/regress/expected/rangetypes.out
index accf1e0d9e..60d875e898 100644
--- a/src/test/regress/expected/rangetypes.out
+++ b/src/test/regress/expected/rangetypes.out
@@ -652,6 +652,30 @@ select daterange('2000-01-10'::date, '2000-01-11'::date, '(]');
[01-11-2000,01-12-2000)
(1 row)
+select daterange('-infinity'::date, '2000-01-01'::date, '()');
+ daterange
+------------------------
+ (-infinity,01-01-2000)
+(1 row)
+
+select daterange('-infinity'::date, '2000-01-01'::date, '[)');
+ daterange
+------------------------
+ [-infinity,01-01-2000)
+(1 row)
+
+select daterange('2000-01-01'::date, 'infinity'::date, '[)');
+ daterange
+-----------------------
+ [01-01-2000,infinity)
+(1 row)
+
+select daterange('2000-01-01'::date, 'infinity'::date, '[]');
+ daterange
+-----------------------
+ [01-01-2000,infinity]
+(1 row)
+
-- test GiST index that's been built incrementally
create table test_range_gist(ir int4range);
create index test_range_gist_idx on test_range_gist using gist (ir);
diff --git a/src/test/regress/sql/rangetypes.sql b/src/test/regress/sql/rangetypes.sql
index 55638a85ee..9fdb1953df 100644
--- a/src/test/regress/sql/rangetypes.sql
+++ b/src/test/regress/sql/rangetypes.sql
@@ -165,6 +165,10 @@ select daterange('2000-01-10'::date, '2000-01-20'::date, '(]');
select daterange('2000-01-10'::date, '2000-01-20'::date, '()');
select daterange('2000-01-10'::date, '2000-01-11'::date, '()');
select daterange('2000-01-10'::date, '2000-01-11'::date, '(]');
+select daterange('-infinity'::date, '2000-01-01'::date, '()');
+select daterange('-infinity'::date, '2000-01-01'::date, '[)');
+select daterange('2000-01-01'::date, 'infinity'::date, '[)');
+select daterange('2000-01-01'::date, 'infinity'::date, '[]');
-- test GiST index that's been built incrementally
create table test_range_gist(ir int4range);
--
2.20.1
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-13 12:44 Thomas Munro <thomas.munro@gmail.com>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Munro @ 2019-07-13 12:44 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-14 03:27 Thomas Munro <thomas.munro@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Munro @ 2019-07-14 03:27 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-18 20:56 Jeff Davis <pgsql@j-davis.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 2 replies; 10+ messages in thread
From: Jeff Davis @ 2019-07-18 20:56 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-18 21:36 Tom Lane <tgl@sss.pgh.pa.us>
parent: Jeff Davis <pgsql@j-davis.com>
1 sibling, 2 replies; 10+ messages in thread
From: Tom Lane @ 2019-07-18 21:36 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-18 22:02 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Jeff Davis <pgsql@j-davis.com>
1 sibling, 0 replies; 10+ messages in thread
From: Laurenz Albe @ 2019-07-18 22:02 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; Thomas Munro <thomas.munro@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-19 00:17 Jeff Davis <pgsql@j-davis.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 10+ messages in thread
From: Jeff Davis @ 2019-07-19 00:17 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-19 00:19 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 10+ messages in thread
From: Michael Paquier @ 2019-07-19 00:19 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jeff Davis <pgsql@j-davis.com>; Thomas Munro <thomas.munro@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190719001919.GA1859@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Bad canonicalization for dateranges with 'infinity' bounds
@ 2019-07-21 18:25 Stephen Frost <sfrost@snowman.net>
parent: Jeff Davis <pgsql@j-davis.com>
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Frost @ 2019-07-21 18:25 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../20190721182529.GG29202@tamriel.snowman.net/2-signature.asc)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2019-07-21 18:25 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 12:40 Bad canonicalization for dateranges with 'infinity' bounds Laurenz Albe <laurenz.albe@cybertec.at>
2019-05-02 12:49 ` Laurenz Albe <laurenz.albe@cybertec.at>
2019-07-13 12:44 ` Thomas Munro <thomas.munro@gmail.com>
2019-07-14 03:27 ` Thomas Munro <thomas.munro@gmail.com>
2019-07-18 20:56 ` Jeff Davis <pgsql@j-davis.com>
2019-07-18 21:36 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-07-19 00:17 ` Jeff Davis <pgsql@j-davis.com>
2019-07-21 18:25 ` Stephen Frost <sfrost@snowman.net>
2019-07-19 00:19 ` Michael Paquier <michael@paquier.xyz>
2019-07-18 22:02 ` Laurenz Albe <laurenz.albe@cybertec.at>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox