On Thu, Nov 23, 2023 at 4:33 AM Richard Guo <guofengli...@gmail.com> wrote: > > On Sun, Nov 19, 2023 at 9:17 AM Alexander Korotkov <aekorot...@gmail.com> > wrote: >> >> It's here. New REALLOCATE_BITMAPSETS forces bitmapset reallocation on >> each modification. > > > +1 to the idea of introducing a reallocation mode to Bitmapset. > >> >> I had the feeling of falling into a rabbit hole while debugging all >> the cases of failure with this new option. With the second patch >> regressions tests pass. > > > It seems to me that we have always had situations where we share the > same pointer to a Bitmapset structure across different places. I do not > think this is a problem as long as we do not modify the Bitmapsets in a > way that requires reallocation or impact the locations sharing the same > pointer. > > So I'm wondering, instead of attempting to avoid sharing pointer to > Bitmapset in all locations that have problems, can we simply bms_copy > the original Bitmapset within replace_relid() before making any > modifications, as I proposed previously? Of course, as Andres pointed > out, we need to do so also for the "Delete relid without substitution" > path. Please see the attached.
Yes, this makes sense. Thank you for the patch. My initial point was that replace_relid() should either do in-place in all cases or make a copy in all cases. Now I see that it should make a copy in all cases. Note, that without making a copy in delete case, regression tests fail with REALLOCATE_BITMAPSETS on. Please, find the revised patchset. As Ashutosh Bapat asked, asserts are split into separate patch. ------ Regards, Alexander Korotkov
0003-Make-replace_relid-leave-argument-unmodified-v2.patch
Description: Binary data
0001-Add-asserts-to-bimapset-manipulation-functions-v2.patch
Description: Binary data
0002-REALLOCATE_BITMAPSETS-manual-compile-time-option-v2.patch
Description: Binary data
