On Fri, 30 Oct 2026 at 21:50, Tomas Vondra <[email protected]> wrote: > > 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?
>From my understanding, numeric_sub_safe would fail only for non-representable difference, which is too many digits in the weight (NUMERIC_WEIGHT(result)). So looks ok > > 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. Thanks. I think your overflow fixes are fine (with nitpicks from Shihao). -- Best regards, Kirill Reshke
