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 |
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 |