Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.

Поиск
Список
Период
Сортировка
От Richard Guo
Тема Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.
Дата
Msg-id CAMbWs49fy=f-jCd7s33d9AGjnYZn6R8TEDU6armi7Yo6QHCKiw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.
Список pgsql-hackers

On Thu, May 9, 2024 at 6:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:
> I'm fine with this one as it's the same as what I already mentioned
> earlier.  I had imagined doing bms_del_member(bms_copy ... but maybe
> the compiler is able to optimise away the additional store. Likely, it
> does not matter much as pallocing memory likely adds far more overhead
> anyway.

I actually wrote it that way to start with, but undid it after
noticing that the existing code in remove_rel_from_restrictinfo
does it in separate steps, and thinking that that was good for
both separation of concerns and a cleaner git history.  I too
can't believe that an extra fetch will be noticeable compared
to the cost of the adjacent bms_xxx operations.

I also think it seems better to do bms_copy in separate steps, not only
because this keeps consistent with the existing code in
remove_rel_from_restrictinfo, but also because we need to do
bms_del_member twice for each lefthand/righthand relid set.

Speaking of consistency, do you think it would improve the code's
readability if we rearrange the code in remove_rel_from_query so that
the modifications of the same relid set are grouped together, just like
what we do in remove_rel_from_restrictinfo?  I mean something like:

   sjinf->min_lefthand = bms_copy(sjinf->min_lefthand);
   sjinf->min_lefthand = bms_del_member(sjinf->min_lefthand, relid);
   sjinf->min_lefthand = bms_del_member(sjinf->min_lefthand, ojrelid);

   ...

Thanks
Richard

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

Предыдущее
От: Peter Smith
Дата:
Сообщение: Re: Slow catchup of 2PC (twophase) transactions on replica in LR
Следующее
От: Noah Misch
Дата:
Сообщение: Re: Weird test mixup