Skip site navigation (1) Skip section navigation (2)

Re: About "Our CLUSTER implementation is pessimal" patch

From: Leonardo F <m_lists(at)yahoo(dot)it>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: About "Our CLUSTER implementation is pessimal" patch
Date: 2010-02-01 11:09:14
Message-ID: 168616.9579.qm@web29010.mail.ird.yahoo.com (view raw or flat)
Thread:
Lists: pgsql-hackers
I know you're all very busy getting 9.0 out, but I think the results in
heap scanning + sort instead of index scanning for CLUSTER are
very good... I would like to know if I did something wrong/I should
improve something in the patch... I haven't tested it with index
expressions yet (but the tests in "make check" work).

Thanks

Leonardo


> Hi all,
> 
> attached a patch to do seq scan + sorting instead of index scan 
> 
> on CLUSTER (when that's supposed to be faster).
> 
> As I've already said, the patch is based on: 
> http://archives.postgresql.org/pgsql-hackers/2008-08/msg01371.php
> 
> Of course, the code isn't supposed to be ready to be merged: I
> would like to write more comments and add some test cases to
> cluster.sql (plus change all the things you are going to tell me I
> have to change...)
> 
> I would like your opinions on code correctness and the decisions
> I took, especially:
> 
> 1) function names ("cost_index_scan_vs_seqscansort" I guess
> it's awful...)
> 
> 2) the fact that I put in Tuplesortstate an EState variable, so that 
> MakeSingleTupleTableSlot wouldn't have to be called for every
> row in the expression indexes case
> 
> 3) the expression index case is not "optimized": I preferred to
> call FormIndexDatum once for the first key value in 
> copytup_rawheap and another time to get all the remaining values
> in comparetup_rawheap. I liked the idea of re-using
> FormIndexDatum in that case, instead of copying&pasting only
> the relevant code: but FormIndexDatum returns all the values,
> 
> even when I might need only the first one
> 
> 
> 4) I refactored the code to deform and rewrite tuple into the function
> "deform_and_rewrite_tuple", because now that part can be called
> by the regular index scan or by the new seq-scan + sort (but I
> could copy&paste those lines instead of refactoring them into a new
> function)
> 
> Suggestions and comments are not just welcome, but needed!


      

Attachment: sorted_cluster.patch
Description: application/octet-stream (26.2 KB)

In response to

pgsql-hackers by date

Next:From: Thom BrownDate: 2010-02-01 12:11:27
Subject: Re: Review: listagg aggregate
Previous:From: Tim BunceDate: 2010-02-01 10:58:46
Subject: Re: Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group