Re: GIT patch review - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: GIT patch review
Date
Msg-id 4654A710.80804@enterprisedb.com
Whole thread Raw
In response to GIT patch review  (Alexey Klyukin <alexk@commandprompt.com>)
List pgsql-hackers
Alexey Klyukin wrote:
> What about proposition for indexam changes, I can't find any patches there ?
> 
> (http://momjian.us/mhonarc/patches/msg00125.html)

That mail is just discussion that lead to the patch below:

> Is the patch for changing amgetmulti to amgetbitmap relevant to the GIT patch ?
> 
> This original patch is here:
> http://archives.postgresql.org/pgsql-patches/2007-03/msg00163.php

The fundamental change to the indexam API that GIT requires is support 
for returning candidate matches. There's two parts to that: the bitmap 
index scan API (amgetmulti) and the regular amgettuple API.

Another issue that needs to be dealt with is that as the GIT patch 
stands, it doesn't retain the ordering of tuples within a heap page, 
which means that they need to be sorted on a page-by-page basis when 
returning them to the executor. That's ugly, the way it's implemented 
now. To make it less ugly, we'd need support in the amgettuple API to 
return tuples in partial ordering.

Since there was discussion on changing the bitmap index API to make it 
more efficient for on-disk bitmap indexes, I created a combined patch 
that both works nicely with on-disk bitmap indexes, and supports 
candidate matches. That's what the amgetmulti->amgetbitmap patch does.


The other part, supporting candidate matches in the amgettuple API 
hasn't been done. I posted a design that's in the patch queue ("Indexam 
interface proposal"), hoping to have a consensus on that. There was some 
discussion on using the candidate matches support for GIN and GiST as 
well. IIRC there was no objections to the candidate matches support, but 
I haven't written a patch to do that.

A more controversial and invasive change is the support for returning 
tuples in partial ordering. If we don't want to do that, we can modify 
the main GIT/clustered indexes patch so that it does retain the full 
ordering of tuples. It'll degrade much more quickly to a normal B-tree 
if tuples are not perfectly ordered on the heap, if tuples are updated 
for examply, but it'll be less invasive.

> It doesn't apply cleanly to the cvs head, I can provide necessary changes
> (actually I've sent them and figured there is a nowhere mentioned limit on
> -hackers which is why I'm resending the message without that patch),

Ok, thanks.

> Any other suggestions on reviewing the GIT patch ?

You might want to start by reading the readme: 
http://community.enterprisedb.com/git/git-readme.txt

Thanks for looking into this. If you have any questions, just ask.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: "George Pavlov"
Date:
Subject: Re: [GENERAL] query log corrupted-looking entries
Next
From: richard@bowmansystems.com
Date:
Subject: Possible to inline setof SQL UDFs?