Fix type of 'reduction' variable in _bt_singleval_fillfactor()

Started by Postgress Cybrosys28 days ago2 messageshackers
Jump to latest
#1Postgress Cybrosys
postgress@cybrosys.com

Hi,

While reviewing nbtdedup.c I noticed a minor type mismatch in
_bt_singleval_fillfactor(). The local variable 'reduction' is declared
as 'int', but it holds the result of multiplying 'leftfree' (which is
type Size, i.e. size_t / unsigned long) by a double factor, and is
then subtracted from state->maxpostingsize, which is also Size.

Using a signed int here is semantically incorrect: Size is the
appropriate type for any variable representing a byte count, and
it matches the type of every other variable involved in this
calculation.

While no overflow occurs with current BLCKSZ limits (the product is
at most ~30KB on a standard build, well within INT_MAX), the type
mismatch could silently produce incorrect behaviour on non-standard
builds compiled with a very large BLCKSZ. In that case, if the
product exceeded INT_MAX, 'reduction' would wrap to a large negative
number. The subsequent check:

if (state->maxpostingsize > reduction)
state->maxpostingsize -= reduction;

would then subtract a negative value, i.e. increase maxpostingsize
instead of reducing it, silently defeating the single-value fill
strategy entirely.

The fix is a one-line change: declare 'reduction' as Size instead
of int.

A patch file is attached.

Thanks & Regards,

*Jhon k*

Postgres Specialist

Project & IT Department

Cybrosys Technologies

Mail

Mobile

WhatsApp

postgress@cybrosys.com

+91 8606827707

+91 8606827707

Attachments:

0001-Fix-type-of-reduction-variable-in-_bt_singleval_fill.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-type-of-reduction-variable-in-_bt_singleval_fill.patchDownload+1-2
In reply to: Postgress Cybrosys (#1)
Re: Fix type of 'reduction' variable in _bt_singleval_fillfactor()

On Thu, Mar 19, 2026 at 1:57 AM Postgress Cybrosys
<postgress@cybrosys.com> wrote:

While no overflow occurs with current BLCKSZ limits (the product is
at most ~30KB on a standard build, well within INT_MAX), the type
mismatch could silently produce incorrect behaviour on non-standard
builds compiled with a very large BLCKSZ.

We don't support BLCKSZ greater than 32KiB. Fields like
ItemIdData.lp_len only have space for 15 bits (independent of BLCKSZ
itself). Many places (likely thousands) rely on that limit.

In that case, if the
product exceeded INT_MAX, 'reduction' would wrap to a large negative
number. The subsequent check:

if (state->maxpostingsize > reduction)
state->maxpostingsize -= reduction;

would then subtract a negative value, i.e. increase maxpostingsize
instead of reducing it, silently defeating the single-value fill
strategy entirely.

This cannot happen with any supported BLCKSZ. Even if we wanted to
support larger BLCKSZ builds, why start here?

--
Peter Geoghegan