Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'

On Mon, Nov 27, 2023 at 6:35 AM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>
> On Fri, Nov 24, 2023 at 3:54 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> >
> > On Thu, Nov 23, 2023 at 4:33 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > >
> > > On Sun, Nov 19, 2023 at 9:17 AM Alexander Korotkov <aekorotkov(at)gmail(dot)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.
>
> Any objections to pushing this?
>

Did we at least measure the memory impact?

How do we ensure that we are not making unnecessary copies of Bitmapsets?

--
Best Wishes,
Ashutosh Bapat