Re: BRIN minmax multi - incorrect distance for infinite timestamp/date - Mailing list pgsql-hackers

From Dean Rasheed
Subject Re: BRIN minmax multi - incorrect distance for infinite timestamp/date
Date
Msg-id CAEZATCWwqTdcBC1ee1uLp+KG4XZLmxFwEGX9y9sBLUR+OdQHeA@mail.gmail.com
Whole thread Raw
In response to BRIN minmax multi - incorrect distance for infinite timestamp/date  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Responses Re: BRIN minmax multi - incorrect distance for infinite timestamp/date
List pgsql-hackers
On Thu, 12 Oct 2023 at 23:43, Tomas Vondra
<tomas.vondra@enterprisedb.com> 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))
>         PG_RETURN_FLOAT8(0);
>

Yes indeed, that looks wrong. I noticed the same thing while reviewing
the infinite interval patch.

> so means infinite values are "very close" to any other value, and thus
> likely to be merged into a summary range. That's exactly the opposite of
> what we want to do, possibly resulting in inefficient indexes.
>

Is this only inefficient? Or can it also lead to wrong query results?

> Attached is a patch fixing this
>

I wonder if it's actually necessary to give infinity any special
handling at all for dates and timestamps. For those types, "infinity"
is actually just INT_MIN/MAX, which compares correctly with any finite
value, and will be much larger/smaller than any common value, so it
seems like it isn't necessary to give "infinite" values any special
treatment. That would be consistent with date_cmp() and
timestamp_cmp().

Something else that looks wrong about that BRIN code is that the
integer subtraction might lead to overflow -- it is subtracting two
integer values, then casting the result to float8. It should cast each
input before subtracting, more like brin_minmax_multi_distance_int8().

IOW, I think brin_minmax_multi_distance_date/timestamp could be made
basically identical to brin_minmax_multi_distance_int4/8.

Regards,
Dean



pgsql-hackers by date:

Previous
From: Benoit Lobréau
Date:
Subject: Re: Questions about the new subscription parameter: password_required
Next
From: "a.rybakina"
Date:
Subject: Re: Removing unneeded self joins