From: | "Greg Stark" <stark(at)enterprisedb(dot)com> |
---|---|
To: | "Gianni Ciolli" <gianni(dot)ciolli(at)2ndquadrant(dot)it> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, "Gabriele Bartolini" <gabriele(dot)bartolini(at)2ndquadrant(dot)it> |
Subject: | Re: Bitmap Indexes patch (was Re: Bitmap Indexes: request for feedback) |
Date: | 2008-11-03 22:37:24 |
Message-ID: | 4136ffa0811031437jb57ebf8p710616e5237c9403@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2008-10-31, Gianni Ciolli <gianni(dot)ciolli(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2008-11-03 22:50:50 | Re: Transactions and temp tables |
Previous Message | Simon Riggs | 2008-11-03 22:06:02 | Re: Hot standby v5 patch assertion failure |