From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Claudio Freire <klaussfreire(at)gmail(dot)com> |
Subject: | Re: Sparse bit set data structure |
Date: | 2019-03-20 17:50:04 |
Message-ID: | CAOBaU_ZUjHoAtLJwzPnPYmeTpv2L0Dt4m0hvKWhQtkTiFqGFug@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Mar 20, 2019 at 5:20 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Wed, Mar 20, 2019 at 2:10 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> > I'm now pretty satisfied with this. Barring objections, I'll commit this
> > in the next few days. Please review, if you have a chance.
>
> You're defining SIMPLE8B_MAX_VALUE but never use it. Maybe you wanted
> to add an assert / explicit test in intset_add_member()?
>
> /*
> * We buffer insertions in a simple array, before packing and inserting them
> * into the B-tree. MAX_BUFFERED_VALUES sets the size of the buffer. The
> * encoder assumes that it is large enough, that we can always fill a leaf
> * item with buffered new items. In other words, MAX_BUFFERED_VALUES must be
> * larger than MAX_VALUES_PER_LEAF_ITEM.
> */
> #define MAX_BUFFERED_VALUES (MAX_VALUES_PER_LEAF_ITEM * 2)
>
> The *2 is not immediately obvious here (at least it wasn't to me),
> maybe explaining intset_flush_buffered_values() main loop rationale
> here could be worthwhile.
>
> Otherwise, everything looks just fine!
I forgot to mention a minor gripe about the intset_binsrch_uint64 /
intset_binsrch_leaf function, which are 99% duplicates. But I don't
know if fixing that (something like passing the array as a void * and
passing a getter function?) is worth the trouble.
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-03-20 17:56:55 | Re: PostgreSQL pollutes the file system |
Previous Message | Julien Rouhaud | 2019-03-20 17:36:13 | Re: PostgreSQL pollutes the file system |