Re: BUG #17876: Function width_bucket() for float8 input returns value out of range

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Mats Kindahl <mats(at)timescale(dot)com>
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
Date: 2023-03-30 15:35:03
Message-ID: 186575.1680190503@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Mats Kindahl <mats(at)timescale(dot)com> writes:
> However, better safe than sorry, so I modified the patch to include the
> check. And yes, you're right in that there is no need to check for the
> operand diff since the previous checks guarantee that the operand is
> between the bounds, and since the diff between the bounds is not infinite,
> the diff between the operand and any of the bounds cannot be infinite.
> Added a comment to that effect to the patch as well.

I looked this over and noted two problems:

* You missed fixing the mirror code path (bound1 > bound2).

* It seems at least possible that, for an operand just slightly less
than bound2, the quotient ((operand - bound1) / (bound2 - bound1))
could round to exactly 1, even though it should theoretically always
be in [0, 1). If that did happen, and count is INT_MAX, then the final
addition of 1 would create its own possibility of integer overflow.
We have code to check that but it's only applied in the operand >= bound2
case. I fixed that by moving the overflow-aware addition of 1 to the
bottom of the function so it's done in all cases, and adjusting the other
code paths to account for that.

Pushed with those changes and some cosmetic tweaking.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2023-03-30 17:33:40 Re: BUG #17877: Referencing a system column in a foreign key leads to incorrect memory access
Previous Message Tom Lane 2023-03-30 11:47:29 Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering