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