On Fri, 29 Dec 2023 at 21:07, Richard Guo <guofenglinux@gmail.com> wrote:
> It seems to me that the former scenario also makes sense in some cases.
> For instance, let's say there are two pointers in two structs, s1->p and
> s2->p, both referencing the same bitmapset. If we modify the bitmapset
> via s1->p (even if no reallocation could occur), s2 would see different
> content via its pointer 'p'.
That statement simply isn't true. If there's no reallocation then
both pointers point to the same memory and, therefore have the same
content, not different content. In the absence of a reallocation,
then the only time s1->p and s2->p could differ is if s1->p became an
empty set as a result of the modification.
> Sometimes this is just wrong and could
> cause problems. If we can force a reallocation for every modification
> of the bitmapset, it'd be much easier to find such bugs.
Whether it's intended or unintended, at least it's consistent,
therefore isn't going to behave differently if the number of
bitmapwords in the set changes. All REALLOCATE_BITMAPSETS does for
bms_int_members(), bms_join() and bms_del_members() is change one
consistent behaviour (we never reallocate) into some other consistent
behaviour (we always reallocate).
If we make REALLOCATE_BITMAPSETS work how I propose in my patch then
the reallocation is just limited to cases where the set *could* be
repalloced by a modification. That change gives us consistent
behaviour as the set is *always* reallocated when it *could* be
reallocated. Making it consistent to me, seems good as a debug
option. Swapping one consistent behaviour for another (as you propose)
seems pointless and more likely to force us to change code that works
perfectly fine today.
In any case, the comments in bms_int_members(), bms_join() and
bms_del_members() are now only true when REALLOCATE_BITMAPSETS is not
defined. Are you proposing we leave those comments outdated? or do you
propose that we mention that the left inputs are not recycled when
building with REALLOCATE_BITMAPSETS? In my view, neither of these is
acceptable as often the primary reason to use something like
bms_int_members() over bms_intersect() is exactly because the left
input is recycled. I don't want people to have to start contorting
code because bms_int_members()'s left input recycling cannot be relied
on.
David
