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

Поиск
Список
Период
Сортировка
От Richard Guo
Тема Re: BUG #18247: Integer overflow leads to negative width
Дата
Msg-id CAMbWs49A6rMO6WD1ECvZUKXuzMxBDyXP6CmJKQoCS5=w-tjL7w@mail.gmail.com
обсуждение исходный текст
Ответ на Re: BUG #18247: Integer overflow leads to negative width  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: BUG #18247: Integer overflow leads to negative width  (Junwang Zhao <zhjwpku@gmail.com>)
Список pgsql-bugs
Дерево обсуждения

On Fri, Dec 15, 2023 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Probably better to clamp tuple width estimates to MaxAllocSize.
>> Anything larger would not correspond to reality anyhow.

> Fair point.  How about the attached patch?

We'd need to hit at least build_joinrel_tlist too.

Right.  And I think we also need to hit add_placeholders_to_joinrel.
 
  Not sure
offhand whether this is enough to cover upper-relation tlists.

I think we need to do the same to create_one_window_path.  For
intermediate WindowAggPaths, we need to add the WindowFuncs to their
output targets, and that would increase the width of the tlists.

Also, it is possible that there could be tuple-width overflow occurring
within function get_rel_data_width().  The return value of this function
is used to calculate density, or returned by get_relation_data_width().
So I wonder if we could change the return type of get_rel_data_width()
and get_relation_data_width() to be double, and handle the overflow in
the callers if needed.  But this may lead to ABI break.
 
As far as the specifics go, is it enough to clamp once?  I think
we'd either have to clamp after each addition, or change the
running-sum variables to double and clamp just before storing
into the final width field.  The latter seems probably
less error-prone in the long run.

Agreed.
 
Also, given that we'll need at least three copies of the clamp
rule, I wonder if it's worth inventing a function comparable
to clamp_row_est().

Yeah, I think we should do that.

Attached is an updated patch for all the changes.  It also changes the
loop_count parameter to be double for compute_bitmap_pages() in passing.
It does not improve the comments for compute_bitmap_pages() though.

Thanks
Richard
Вложения

В списке pgsql-bugs по дате отправления:

Предыдущее
От: Richard Guo
Дата:
Сообщение: Re: BUG #18247: Integer overflow leads to negative width
Следующее
От: Junwang Zhao
Дата:
Сообщение: Re: BUG #18247: Integer overflow leads to negative width