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

On Tue, Dec 19, 2023 at 5:00 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

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

I agree with all the comments made. I have also examined other places
where the width field is assigned, and I think we've covered all cases.
So the v3 patch is in good shape to me.

Thanks
Richard