public inbox for [email protected]  
help / color / mirror / Atom feed
Fix type of 'reduction' variable in _bt_singleval_fillfactor()
2+ messages / 2 participants
[nested] [flat]

* Fix type of 'reduction' variable in _bt_singleval_fillfactor()
@ 2026-03-19 05:57 Postgress Cybrosys <[email protected]>
  2026-03-19 22:09 ` Re: Fix type of 'reduction' variable in _bt_singleval_fillfactor() Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 2+ messages in thread

From: Postgress Cybrosys @ 2026-03-19 05:57 UTC (permalink / raw)
  To: pgsql-hackers

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


[email protected]

+91 8606827707

+91 8606827707


Attachments:

  [text/x-patch] 0001-Fix-type-of-reduction-variable-in-_bt_singleval_fill.patch (1.4K, 3-0001-Fix-type-of-reduction-variable-in-_bt_singleval_fill.patch)
  download | inline diff:
From b6797809a22f44841ca17f3bb750b71545818670 Mon Sep 17 00:00:00 2001
From: Cybrosys <[email protected]>
Date: Thu, 19 Mar 2026 11:22:08 +0530
Subject: [PATCH] Fix type of reduction variable in _bt_singleval_fillfactor

The local variable 'reduction' in _bt_singleval_fillfactor() was
declared as int, but it holds the result of multiplying leftfree
(which is Size, i.e. size_t / unsigned long) by a double factor.

Using a signed int is a type mismatch: Size is the correct type for
any quantity representing a byte count, and matches the type of the
variable it is subtracted from (maxpostingsize).

In practice no overflow occurs with current BLCKSZ limits, but the
wrong type is misleading and could silently misbehave on non-standard
builds with very large BLCKSZ values.
---
 src/backend/access/nbtree/nbtdedup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/nbtree/nbtdedup.c b/src/backend/access/nbtree/nbtdedup.c
index 08884116aec..61bf877d5d9 100644
--- a/src/backend/access/nbtree/nbtdedup.c
+++ b/src/backend/access/nbtree/nbtdedup.c
@@ -822,7 +822,7 @@ static void
 _bt_singleval_fillfactor(Page page, BTDedupState state, Size newitemsz)
 {
 	Size		leftfree;
-	int			reduction;
+	Size		reduction;
 
 	/* This calculation needs to match nbtsplitloc.c */
 	leftfree = PageGetPageSize(page) - SizeOfPageHeaderData -
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: Fix type of 'reduction' variable in _bt_singleval_fillfactor()
  2026-03-19 05:57 Fix type of 'reduction' variable in _bt_singleval_fillfactor() Postgress Cybrosys <[email protected]>
@ 2026-03-19 22:09 ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Peter Geoghegan @ 2026-03-19 22:09 UTC (permalink / raw)
  To: Postgress Cybrosys <[email protected]>; +Cc: pgsql-hackers

On Thu, Mar 19, 2026 at 1:57 AM Postgress Cybrosys
<[email protected]> 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





^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2026-03-19 22:09 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-19 05:57 Fix type of 'reduction' variable in _bt_singleval_fillfactor() Postgress Cybrosys <[email protected]>
2026-03-19 22:09 ` Peter Geoghegan <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox