Re: Making empty Bitmapsets always be NULL

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: Yuya Watari <watari.yuya@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Date: 2023-06-20T04:16:56Z
Lists: pgsql-hackers

Attachments

On Thu, 15 Jun 2023 at 20:57, Yuya Watari <watari.yuya@gmail.com> wrote:
>
> On Tue, Jun 13, 2023 at 8:07 PM David Rowley <dgrowleyml@gmail.com> wrote:
> > For the fix in the 0004 patch, I think we can do what you did more
> > simply.  I don't think there's any need to perform the loop to find
> > the last non-zero word.  We're only deleting a member from a single
> > word here, so we only need to check if that word is the last word and
> > remove it if it's become zero.  If it's not the last word then we
> > can't remove it as there must be some other non-zero word after it.
>
> If my thinking is correct, the do-while loop I added is still
> necessary. Consider the following code. The Assertion in this code
> passes in the master but fails in the new patch.
>
> =====
> Bitmapset *x = bms_make_singleton(1000);
>
> x = bms_del_member(x, 1000);
> Assert(x == NULL);
> =====

I'm not sure what I was thinking there.  Yeah, you're right, we do
need to do the backwards loop over the set to trim off the trailing
zero words.

I've adjusted the attached patch to do that.

David

Commits

  1. Remove trailing zero words from Bitmapsets

  2. Remove local optimizations of empty Bitmapsets into null pointers.

  3. Require empty Bitmapsets to be represented as NULL.

  4. Mop up some undue familiarity with the innards of Bitmapsets.

  5. Remove bms_first_member().