Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-29 Thread Ashutosh Bapat
On Fri, Oct 27, 2023 at 10:32 PM Tomas Vondra wrote: > > FWIW I've cleaned up and pushed all the patches we came up with this > thread. And I've backpatched all of them to 14+. > Thanks a lot Tomas. -- Best Wishes, Ashutosh Bapat

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-27 Thread Tomas Vondra
FWIW I've cleaned up and pushed all the patches we came up with this thread. And I've backpatched all of them to 14+. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-22 Thread Tomas Vondra
On 10/20/23 11:52, Ashutosh Bapat wrote: > On Thu, Oct 19, 2023 at 6:11 PM Ashutosh Bapat > wrote: >> >> I think we should provide generate_series(date, date, integer) which >> will use date + integer -> date. > > Just to be clear, I don't mean that this patch should add it. > I'm not against a

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-20 Thread Ashutosh Bapat
On Thu, Oct 19, 2023 at 6:11 PM Ashutosh Bapat wrote: > > I think we should provide generate_series(date, date, integer) which > will use date + integer -> date. Just to be clear, I don't mean that this patch should add it. -- Best Wishes, Ashutosh Bapat

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Ashutosh Bapat
On Thu, Oct 19, 2023 at 4:51 PM Tomas Vondra wrote: > > On 10/19/23 11:22, Ashutosh Bapat wrote: > > On Thu, Oct 19, 2023 at 2:31 PM Tomas Vondra > > wrote: > > > >> > >> Does that explain the algorithm? I'm not against clarifying the comment, > >> of course. > > > > Thanks a lot for this explana

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Tomas Vondra
On 10/19/23 11:22, Ashutosh Bapat wrote: > On Thu, Oct 19, 2023 at 2:31 PM Tomas Vondra > wrote: > >> >> Does that explain the algorithm? I'm not against clarifying the comment, >> of course. > > Thanks a lot for this explanation. It's clear now. > >> I tried to do that, but I ran into troubles

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Ashutosh Bapat
On Thu, Oct 19, 2023 at 2:31 PM Tomas Vondra wrote: > > Does that explain the algorithm? I'm not against clarifying the comment, > of course. Thanks a lot for this explanation. It's clear now. > I tried to do that, but I ran into troubles with the "date" tests. I > needed to build values that c

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Tomas Vondra
On 10/19/23 09:04, Dean Rasheed wrote: > On Thu, 19 Oct 2023, 05:32 Ashutosh Bapat, > wrote: > > On Wed, Oct 18, 2023 at 8:23 PM Tomas Vondra > > wrote: > > > > > I did use that many values to actu

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Tomas Vondra
On 10/19/23 06:32, Ashutosh Bapat wrote: > On Wed, Oct 18, 2023 at 8:23 PM Tomas Vondra > wrote: > >> >> I did use that many values to actually force "compaction" and merging of >> points into ranges. We only keep 32 values per page range, so with 2 >> values we'll not build a range. You're ri

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-19 Thread Dean Rasheed
On Thu, 19 Oct 2023, 05:32 Ashutosh Bapat, wrote: > On Wed, Oct 18, 2023 at 8:23 PM Tomas Vondra > wrote: > > > > > I did use that many values to actually force "compaction" and merging of > > points into ranges. We only keep 32 values per page range, so with 2 > > values we'll not build a range

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Ashutosh Bapat
On Wed, Oct 18, 2023 at 8:23 PM Tomas Vondra wrote: > > I did use that many values to actually force "compaction" and merging of > points into ranges. We only keep 32 values per page range, so with 2 > values we'll not build a range. You're right it may still trigger the > overflow (we probably s

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Tomas Vondra
On 10/18/23 12:47, Ashutosh Bapat wrote: > On Wed, Oct 18, 2023 at 1:55 AM Tomas Vondra > wrote: >> >> Hi, >> >> Here's a couple cleaned-up patches fixing the various discussed here. >> I've tried to always add a regression test demonstrating the issue >> first, and then fix it in the next patch.

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Tomas Vondra
On 10/18/23 12:13, Dean Rasheed wrote: > On Tue, 17 Oct 2023 at 21:25, Tomas Vondra > wrote: >> >> Here's a couple cleaned-up patches fixing the various discussed here. >> I've tried to always add a regression test demonstrating the issue >> first, and then fix it in the next patch. >> > > Thi

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Dean Rasheed
On Wed, 18 Oct 2023 at 09:16, Tomas Vondra wrote: > > BTW when adding the tests with extreme values, I noticed this: > > test=# select '5874897-01-01'::date; >date > --- >5874897-01-01 > (1 row) > > test=# select '5874897-01-01'::date + '1 second'::interval; > ERR

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Ashutosh Bapat
On Wed, Oct 18, 2023 at 1:55 AM Tomas Vondra wrote: > > Hi, > > Here's a couple cleaned-up patches fixing the various discussed here. > I've tried to always add a regression test demonstrating the issue > first, and then fix it in the next patch. It will be good to commit the test changes as well

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Dean Rasheed
On Tue, 17 Oct 2023 at 21:25, Tomas Vondra wrote: > > Here's a couple cleaned-up patches fixing the various discussed here. > I've tried to always add a regression test demonstrating the issue > first, and then fix it in the next patch. > This looks good to me. > 2) incorrect subtraction in dist

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-18 Thread Tomas Vondra
On 10/17/23 22:25, Tomas Vondra wrote: > Hi, > > Here's a couple cleaned-up patches fixing the various discussed here. > I've tried to always add a regression test demonstrating the issue > first, and then fix it in the next patch. > > In particular, this deals with these issues: > > 1) overflow

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-17 Thread Tomas Vondra
Hi, Here's a couple cleaned-up patches fixing the various discussed here. I've tried to always add a regression test demonstrating the issue first, and then fix it in the next patch. In particular, this deals with these issues: 1) overflows in distance calculation for large timestamp values (000

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-16 Thread Ashutosh Bapat
On Mon, Oct 16, 2023 at 7:33 PM Tomas Vondra wrote: > > On 10/16/23 11:25, Ashutosh Bapat wrote: > > Thanks Tomas for bringing this discussion to hackers. > > > > > > On Fri, Oct 13, 2023 at 8:58 PM Dean Rasheed > > wrote: > >> > >> On Fri, 13 Oct 2023 at 13:17, Tomas Vondra > >> wrote: > >>> >

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-16 Thread Tomas Vondra
On 10/16/23 11:25, Ashutosh Bapat wrote: > Thanks Tomas for bringing this discussion to hackers. > > > On Fri, Oct 13, 2023 at 8:58 PM Dean Rasheed wrote: >> >> On Fri, 13 Oct 2023 at 13:17, Tomas Vondra >> wrote: >>> >>> I do plan to backpatch this, yes. I don't think there are many people >>>

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-16 Thread Ashutosh Bapat
Thanks Tomas for bringing this discussion to hackers. On Fri, Oct 13, 2023 at 8:58 PM Dean Rasheed wrote: > > On Fri, 13 Oct 2023 at 13:17, Tomas Vondra > wrote: > > > > I do plan to backpatch this, yes. I don't think there are many people > > affected by this (few people are using infinite dat

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-13 Thread Dean Rasheed
On Fri, 13 Oct 2023 at 13:17, Tomas Vondra wrote: > > I do plan to backpatch this, yes. I don't think there are many people > affected by this (few people are using infinite dates/timestamps, but > maybe the overflow could be more common). > OK, though I doubt that such values are common in pract

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-13 Thread Tomas Vondra
On 10/13/23 14:04, Dean Rasheed wrote: > On Fri, 13 Oct 2023 at 11:44, Tomas Vondra > wrote: >> >> On 10/13/23 11:21, Dean Rasheed wrote: >>> >>> Is this only inefficient? Or can it also lead to wrong query results? >> >> I don't think it can produce incorrect results. It only affects which >> val

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-13 Thread Dean Rasheed
On Fri, 13 Oct 2023 at 11:44, Tomas Vondra wrote: > > On 10/13/23 11:21, Dean Rasheed wrote: > > > > Is this only inefficient? Or can it also lead to wrong query results? > > I don't think it can produce incorrect results. It only affects which > values we "merge" into an interval when building th

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-13 Thread Tomas Vondra
On 10/13/23 11:21, Dean Rasheed wrote: > On Thu, 12 Oct 2023 at 23:43, Tomas Vondra > wrote: >> >> Ashutosh Bapat reported me off-list a possible issue in how BRIN >> minmax-multi calculate distance for infinite timestamp/date values. >> >> The current code does this: >> >> if (TIMESTAMP_NOT

Re: BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-13 Thread Dean Rasheed
On Thu, 12 Oct 2023 at 23:43, Tomas Vondra wrote: > > Ashutosh Bapat reported me off-list a possible issue in how BRIN > minmax-multi calculate distance for infinite timestamp/date values. > > The current code does this: > > if (TIMESTAMP_NOT_FINITE(dt1) || TIMESTAMP_NOT_FINITE(dt2)) >

BRIN minmax multi - incorrect distance for infinite timestamp/date

2023-10-12 Thread Tomas Vondra
Hi, Ashutosh Bapat reported me off-list a possible issue in how BRIN minmax-multi calculate distance for infinite timestamp/date values. The current code does this: if (TIMESTAMP_NOT_FINITE(dt1) || TIMESTAMP_NOT_FINITE(dt2)) PG_RETURN_FLOAT8(0); so means infinite values are "very cl