Re: Fix BRIN numeric_minmax_multi_ops out of range value range merging

On 10/29/26 05:03, Kirill Reshke wrote:
> On Thu, 29 Oct 2026 at 04:05, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>>
>>
>>
>> On 10/28/26 08:02, Kirill Reshke wrote:
>>> On Wed, 28 Oct 2026 at 01:59, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>>>>
>>>> On 10/27/26 19:59, Kirill Reshke wrote:
>>>>> Hi
>>>>> While reviewing some reports in pgsql-bugs I bumped into simple SQL
>>>>> reproduced of BRIN out of range issue
>>>>>
>>>>> CREATE TABLE t (n numeric);
>>>>> INSERT INTO t VALUES (1), (1e1000);
>>>>> CREATE INDEX ON t USING brin (n numeric_minmax_multi_ops);
>>>>>
>>>>> This fails. brin_minmax_multi_distance_numeric computes heuristic, so
>>>>> it can be approximated. Patch uses numeric_sub_safe with an
>>>>> ErrorSaveContext and returns inf if any error occurred (which can only
>>>>> be an overflow in current code base, see make_result_safe)
>>>>>
>>>>> The numeric-distance overflow risk was discussed during the original
>>>>> minmax-multi development in 2018 [0], with normalizing value into 0-1
>>>>> range. I didn't fix that way, the issue fixed in the current
>>>>> implementation.
>>>>>
>>>>
>>>> Thanks for the report. I'll take a closer look at the fix, but it seems
>>>> like the right approach to me.
>>>
>>> Thanks for looking.
>>>
>>>> I wonder if we need to do anything for the other distance functions.
>>>
>>> Looks like there is an same issue with numrange_subdiff, like
>>>
>>> postgres=# SELECT numrange_subdiff('1e1000'::numeric, '2e1000'::numeric);
>>> ERROR: "-10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
>>> is out of range for type double precision
>>>
>>> Attached fixes numrange_subdiff in the same way as brin numeric distance.
>>>
>>> numrange_subdiff is called from the planner without try/catch or
>>> anything, so this overflow impact should be query failure, not index
>>> build failure.
>>>
>>> I did not however reproduced query failure, because of ANALYZE failure:
>>>
>>> ```
>>> postgres=# CREATE TABLE t2 (r nummultirange);
>>> CREATE TABLE
>>> postgres=# INSERT INTO t2 VALUES (
>>> '{[1e1000, 2e1000], [-9e131071, 9e131071]}'::nummultirange
>>> );
>>> INSERT 0 1
>>> postgres=# ANALYZE t;
>>> ERROR: value overflows numeric format
>>> ```
>>>
>>> so a table with extreme numeric bounds cannot be analyzed at all.
>>>
>>
>> Ah, but that's completely unrelated to the BRIN code, right? I was
>> puzzled for a bit, because I don't think we have minmax-multi opclasses
>> for [multi]ranges.
>
> Yes, but you were wondering about other distance functions, so I
> started to search for alike problems... and found one that is neither
> BRIN nor distance, my bad, should be a separate thread. The only thing
> they share is how fix looks - almost the same code to be changed .
>

Yup. It's a good catch, it just confused me for a bit.

I spent a bit of time polishing this up today, I'm attaching two patch
series - one for 19-master, one for 18 (and older). The numeric_sub_safe
was introduced in 19, older branches need to use numeric_sub_opt_error.

Now that I'm thinking about it, are the numeric overflow fixes actually
correct? They do this:

d = numeric_sub_safe(a2, a1, (Node *) &escontext);
if (SOFT_ERROR_OCCURRED(&escontext))
distance = get_float8_infinity();

But doesn't that simply mean the value does not fit into numeric, which
means some number of digits before/after decimal point. Can't it happen
that we end up with too small difference? Maybe that can't happen for
subtraction?

While looking at the BRIN minmax-multi code, I found two other issues in
the distance calculation for TID and OID. Those are less serious, as it
won't break the index, it just makes it less efficient (by constructing
ranges that are very wide).

For "tid", the item pointer -> double mapping was done in uint32, so it
wrapped every ~112GB or so. That was easy to fix by switching to double
arithmetics. There's also a minor issue with using MaxHeapTuplesPerPage,
which the tids may not respect. I didn't do anything about it, it seems
very unlikely in practice.

For OID, the issue is that it used "int4" distance, but that treats
values as int32 and not uint32 (as it should, for OIDs). The proper fix
needs a new pg_amproc entry, so can't be backpatched.

Both the TID/OID cases seem very low impact, few people are using BRIN
on those data types. And it merely makes the indexes less efficient (for
rare cases), not wrong.

regards

--
Tomas Vondra