From: Tom Lane <tgl@sss.pgh.pa.us>
To: Gavin Sherry <swm@linuxworld.com.au>
Cc: Jie Zhang <jzhang@greenplum.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: On-disk bitmap index patch
Date: Sun, 23 Jul 2006 19:43:35 -0400
Message-ID: <28525.1153698215@sss.pgh.pa.us> (raw)
In-Reply-To: <Pine.LNX.4.58.0607240720180.19004@linuxworld.com.au>
References: <C0E1CD9D.8DC0%jzhang@greenplum.com>
<Pine.LNX.4.58.0607240720180.19004@linuxworld.com.au>
Gavin Sherry <swm@linuxworld.com.au> writes:
> Is anyone else looking at this patch?
It's on my list of things to look at, but so are a lot of other patches ;-)
A couple of comments after a *very* fast scan through the patch:
* The xlog routines need help; they seem to not be updated for recent
changes in the API for xlog recovery code.
* 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.
* 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.
* Why in the world is it introducing duplicate copies of a lot
of btree comparison functions? Use the ones that are there.
* The patch itself is a mess: it introduces .orig and .rej files,
changes around $PostgreSQL$ lines, etc.
However, the main problem I've got with this is that a new index AM is a
pretty large burden, and no one's made the slightest effort to sell
pghackers on taking this on. What's the use-case, what's the
performance like, where is it really an improvement over what we've got?
regards, tom lane
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: tgl@sss.pgh.pa.us, swm@linuxworld.com.au, jzhang@greenplum.com
Subject: Re: On-disk bitmap index patch
In-Reply-To: <28525.1153698215@sss.pgh.pa.us>
* 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