Re: Bitmap Indexes patch (was Re: Bitmap Indexes: request for feedback)

On 2008-10-31, Gianni Ciolli <gianni.ciolli@2ndquadrant.it> wrote:

>  following the useful feedback that we received from this list, we
>  would like to submit the patch for Bitmap Indexes for the november
>  CommitFest (joint work of me with Gabriele Bartolini, starting from
>  Gavin Sherry's patch).

I skimmed through this on the plane -- I say "skimmed" because it had
to be pretty quick before the battery ran out :(

I have some first reactions but I admit these are pretty trivial
detail points. I'm still trying to get a good feel for the overall
structure which I fear is where any substantial feedback would come
from.

Firstly, there are a lot of pieces of #ifdef NOTUSED or #if 0 code
which seem to be remnants of Gavin's code which are no longer
relevant. That's pretty trivial for a committer to strip out but if
you cut another patch it would be appreciated if you removed all that
crud.

Secondly the locking seems to be a bit overoptimistic. I'm pretty sure
you have to take an exclusive lock on an index page any time you make
any data modifications in index pages -- even if you're just setting a
bit and not moving any data around. If two processes set two bits in
the same word one can get lost in the race condition.

There are a lot of comments in the code which imply that vacuuming is
not implemented but in fact from what I can see it is -- sort of. It
does rewrite the bitmap in bmbulkdelete but it doesn't have to rebuild
the index from scratch.  Are the comments out of date or am i
misunderstanding them or the code? How complete is the vacuum
implementation?

-- 
greg