From: Jie Zhang <jzhang@greenplum.com>
To: Gavin Sherry <swm@linuxworld.com.au>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, "Luke Lonergan" <LLonergan@greenplum.com>
Subject: Re: On-disk bitmap index patch
Date: Sun, 23 Jul 2006 23:23:48 -0700
Message-ID: <C0E9B584.8F43%jzhang@greenplum.com> (raw)
In-Reply-To: <Pine.LNX.4.58.0607240952250.19854@linuxworld.com.au>
Thanks Tom and Gavin for your comments!
Yes, this patch is generated against 8.2 in a short time. As long as the
code is working, I post the patch to get some comments and help.
>>
>> * The xlog routines need help; they seem to not be updated for recent
>> changes in the API for xlog recovery code.
>
> Yep. The patch was actually against 8.1 and was hastily brought up to 8.2.
> I think Jie's intention was to simply let everyone know that this was
> going on.
Thanks for pointing this out. I didn't notice that these are changed in 8.2.
>
>>
>> * The hacks on vacuum.c (where it tests the AM name) are utterly
>> unacceptable. If you need the main code to do something special for a
>> particular index AM, define a bool flag column for it in pg_am.
>
> Yes.
Sounds good.
>
>>
>> * The interface to the existing executor bitmap scan code is mighty
>> messy --- seems like there's a lot of almost duplicate code, a lot
>> of rather invasive hacking, etc. This needs to be rethought and
>> refactored.
>
> I agree.
I will think about this more.
>
>>
>> * Why in the world is it introducing duplicate copies of a lot
>> of btree comparison functions? Use the ones that are there.
>
> Yes, I raised this with Jie and she has fixed it. One thought is, we may
> want to rename those comparison functions prefixed with 'bm' to make their
> naming less confusing. They'll be used by btree, gin and bitmap index
> methods. Anyway, a seperate patch.
Yeah, the main problem I hesitated to use btree's comparison functions
because of those function names starting with 'bt'. Since Gavin told me that
Gin is using those functions as well, I had changed them. Renaming them
would be good.
>
>>
>> * The patch itself is a mess: it introduces .orig and .rej files,
>> changes around $PostgreSQL$ lines, etc.
>>
>
> Right, not to mention patches to configure and a lot of style which needs
> to be knocked into shape.
>
The way I generate a patch is kind of clumsy. I need to find a better way to
do that.
I will start fixing these.
Thanks,
Jie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: jzhang@greenplum.com, swm@linuxworld.com.au, tgl@sss.pgh.pa.us, LLonergan@greenplum.com
Subject: Re: On-disk bitmap index patch
In-Reply-To: <C0E9B584.8F43%jzhang@greenplum.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox