On Thu, 12 Oct 2023 at 23:43, Tomas Vondra <tomas.von...@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
