> 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...)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
