Unix Technical Forum

GIT patch review

This is a discussion on GIT patch review within the pgsql Hackers forums, part of the PostgreSQL category; --> Hello, I'd like to help reviewing patches, in particular the group index tupes (GIT) patch by Heikki Linnakangas ( ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > pgsql Hackers

FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-12-2008, 09:50 AM
Alexey Klyukin
 
Posts: n/a
Default GIT patch review

Hello,

I'd like to help reviewing patches, in particular the group index tupes (GIT)
patch by Heikki Linnakangas
(http://archives.postgresql.org/pgsql.../msg01264.php).
I've spoken with Alvaro about it, he gave me several links to the threads on
hackers related to the GIT patch and I have some questions:

What about proposition for indexam changes, I can't find any patches there ?

(http://momjian.us/mhonarc/patches/msg00125.html)

Is the patch for changing amgetmulti to amgetbitmap relevant to the GIT patch ?

This original patch is here:
http://archives.postgresql.org/pgsql...3/msg00163.php

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),

Any other suggestions on reviewing the GIT patch ?

Regards,
--
Alexey Klyukin http://www.commandprompt.com/
The PostgreSQL Company - Command Prompt, Inc.




---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-12-2008, 09:50 AM
Heikki Linnakangas
 
Posts: n/a
Default Re: GIT patch review

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...3/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

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 06:54 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com