Re: Strange Bitmapset manipulation in DiscreteKnapsack()

On Thu, 18 Jan 2024 at 14:58, Andy Fan <zhihuifan1213(at)163(dot)com> wrote:
> I want to know if "user just want to zero out the flags in bitmapset
> but keeping the memory allocation" is a valid requirement. Commit
> 00b41463c makes it is hard IIUC.

Looking at your patch, I see:

+/*
+ * does this break commit 00b41463c21615f9bf3927f207e37f9e215d32e6?
+ * but I just found alloc memory and free the memory is too bad
+ * for this current feature. So let see ...;
+ */
+void
+bms_zero(Bitmapset *a)

I understand the requirement here, but, to answer the question in the
comment -- Yes, that does violate the requirements for how an empty
set is represented and as of c7e5e994b and possibly earlier, any
subsequent Bitmapset operations will cause an Assert failure due to
the trailing zero word(s) left by bms_zero().

> Looks like this is another user case of "user just wants to zero out the
> flags in bitmapset but keeping the memory allocation".

I don't really see a way to have it work the way you want without
violating the new representation of an empty Bitmapset. Per [1],
there's quite a performance advantage to 00b41463c plus the additional
don't allow trailing empty words rule done in a8c09daa8. So I don't
wish the rules were more lax.

Maybe Bitmapsets aren't the best fit for your need. Maybe it would be
better to have a more simple fixed size bitset that you could allocate
in the same allocation as the TupleTableSlot's tts_null and tts_values
arrays. The slot's TupleDesc should know how many bits are needed.

David

[1] https://postgr.es/m/CAJ2pMkYcKHFBD_OMUSVyhYSQU0-j9T6NZ0pL6pwbZsUCohWc7Q%40mail.gmail.com