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

Re: I: About "Our CLUSTER implementation is pessimal" patch

From: Leonardo F <m_lists(at)yahoo(dot)it>
To: Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: Josh Kupershmidt <schmiddy(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: I: About "Our CLUSTER implementation is pessimal" patch
Date: 2010-07-06 09:31:39
Message-ID: 691506.78032.qm@web29014.mail.ird.yahoo.com (view raw or flat)
Thread:
Lists: pgsql-hackers
> I reviewed 
> your patch. It seems to be in good shape, and worked as
> expected. I 
> suppressed a compiler warning in the patch and cleaned up
> whitespaces in it. 
> Patch attached.


Thanks for the review!

I saw that you also changed the writing:

 LogicalTapeWrite(state->tapeset, tapenum,
 tuple, HEAPTUPLESIZE);
 LogicalTapeWrite(state->tapeset, tapenum, tuple->t_data, tuple->t_len-HEAPTUPLESIZE);


and the reading:

 tuple->t_len = tuplen - HEAPTUPLESIZE;
 if (LogicalTapeRead(state->tapeset, tapenum, &tuple->t_self, tuplen-sizeof(tuplen)) != tuplen-sizeof(tuplen))
  elog(ERROR, "unexpected end of data");



into (writing):

 LogicalTapeWrite(state->tapeset, tapenum,
    tuple, tuple->t_len);


(reading):

if (LogicalTapeRead(state->tapeset, tapenum, &tuple->t_self, tuplen-sizeof(tuplen)) != tuplen-sizeof(tuplen))



Are we sure it's 100% equivalent?

I remember I had issues with the fact that tuple->t_data wasn't
at HEAPTUPLESIZE distance from tuple:

http://osdir.com/ml/pgsql-hackers/2010-02/msg00744.html


> I think we need some documentation for the change. The 
> only downside
> of the feature is that sorted cluster requires twice disk 
> spaces of
> the target table (temp space for disk sort and the result 
> table).
> Could I ask you to write documentation about the new 
> behavior?
> Also, code comments can be improved; especially we need 
> better
> description than "copy&paste from 
> FormIndexDatum".


I'll try to improve the comments and add doc changes (but my English
will have to be double checked...)



      

In response to

Responses

pgsql-hackers by date

Next:From: Robert HaasDate: 2010-07-06 13:23:40
Subject: Re: get_whatever_oid, part 2
Previous:From: Takahiro ItagakiDate: 2010-07-06 07:52:27
Subject: Re: I: About "Our CLUSTER implementation is pessimal" patch

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