Re: BUG #18247: Integer overflow leads to negative width

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Junwang Zhao <zhjwpku(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, rekgrpth(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #18247: Integer overflow leads to negative width
Date: 2023-12-18 21:00:30
Message-ID: 4099430.1702933230@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Junwang Zhao <zhjwpku(at)gmail(dot)com> writes:
> I guess using double for the sum is in case of overflow of int64?
> pg_class.relnatts
> is of type int16, I guess it's not possible to overflow int64.

Not sure what I think about that. The main advantage of double is
that you definitely don't need to worry about overflow. It's
probably about the same speed either way on modern hardware.
(On 32-bit hardware it's quite likely that double is faster,
since even 32-bit CPUs tend to have hardware floating point.)
I left it as double for the moment, but we could discuss that
some more.

Some review comments:

* clamp_width_est doesn't need to check isinf, since the
comparison will work just fine. It does need to check isnan,
unless we switch to int64.

* We can fold the Assert about width >= 0 into clamp_width_est
too; it fits naturally there and saves code in the callers.

* I did not like changing the output type of get_rel_data_width;
it seems better to me to have it do clamp_width_est internally.
For one reason, several of the call sites are relying on
doing integer division, which the v2 patch might break.

The attached v3 fixes those things and makes some cosmetic
changes (mostly comments).

regards, tom lane

Attachment Content-Type Size
v3-0001-Clamp-tuple_width-to-MaxAllocSize.patch text/x-diff 7.4 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Richard Guo 2023-12-19 03:32:29 Re: BUG #18247: Integer overflow leads to negative width
Previous Message PG Bug reporting form 2023-12-18 16:00:02 BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term