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: pgsql-hackers(at)postgresql(dot)org
Subject: Re: I: About "Our CLUSTER implementation is pessimal" patch
Date: 2010-02-10 09:11:40
Message-ID: 482293.44393.qm@web29004.mail.ird.yahoo.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Hi all,


while testing the seq scan + sort CLUSTER implementation, I've found a
bug in write/readtup_rawheap.

The functions are needed by the sort part.
The code in 

http://archives.postgresql.org/pgsql-hackers/2008-08/msg01371.php

didn't copy the whole tuple, but only the HeapTuple "header": the t_data
part wasn't copied (at least, that's my understanding), The original code
is at the bottom of the email. It didn't work (the table wasn't fully clustered
at the end of the CLUSTER command).

So I modified write/readtup_rawheap: they are supposed to store/retrieve
both the "fixed" part of HeapTupleData, plus the "variable" part t_data.
But a get a lot of:
WARNING:  problem in alloc set TupleSort: detected write past chunk end
  in block 0x96853f0, chunk 0x968723c
WARNING:  problem in alloc set TupleSort: detected write past chunk end 
  in block 0x96853f0, chunk 0x96872c8
warnings when calling "tuplesort_end"  and some of the data gets garbled
after the CLUSTER command.


What am I doing wrong? Using the debugger, data coming out of
readtup_rawheap seems fine...

I *really* need your help here...


static void
writetup_rawheap(Tuplesortstate *state, int tapenum, SortTuple *stup)
{
HeapTuple    tuple = (HeapTuple) stup->tuple;
tuple->t_len += sizeof(HeapTupleData); /* write out the header as well */

LogicalTapeWrite(state->tapeset, tapenum,
 				    tuple, sizeof(HeapTupleData));
LogicalTapeWrite(state->tapeset, tapenum, tuple->t_data, 
     tuple->t_len-sizeof(HeapTupleData));
if (state->randomAccess)    /* need trailing length word? */
          LogicalTapeWrite(state->tapeset, tapenum,
                             tuple, sizeof(tuple->t_len));

FREEMEM(state, GetMemoryChunkSpace(tuple));
heap_freetuple(tuple);
}

static void
readtup_rawheap(Tuplesortstate *state, SortTuple *stup,
int tapenum, unsigned int tuplen)
{
HeapTuple    tuple = (HeapTuple) palloc(sizeof(HeapTupleData));
tuple->t_data = (HeapTupleHeader) palloc(tuplen-sizeof(HeapTupleData));

USEMEM(state, GetMemoryChunkSpace(tuple));

tuple->t_len = tuplen - sizeof(HeapTupleData);
if (LogicalTapeRead(state->tapeset, tapenum, &tuple->t_self, sizeof(HeapTupleData)-sizeof(tuplen))
       != sizeof(HeapTupleData)-sizeof(tuplen))
          elog(ERROR, "unexpected end of data");
if (LogicalTapeRead(state->tapeset, tapenum, tuple->t_data, tuple->t_len) != tuple->t_len)
          elog(ERROR, "unexpected end of data");
if (state->randomAccess)    /* need trailing length word? */
         if (LogicalTapeRead(state->tapeset, tapenum, &tuplen,
                               sizeof(tuplen)) != sizeof(tuplen))
                  elog(ERROR, "unexpected end of data");

stup->tuple = tuple;
/* set up first-column key value */
if (state->indexInfo->ii_Expressions == NULL)
{
/* no expression index, just get the key datum value */
stup->datum1 = heap_getattr((HeapTuple) stup->tuple,
state->indexInfo->ii_KeyAttrNumbers[0],
state->tupDesc,
&stup->isnull1);
}
else
{
  [...] expression index part, removed for clarity     
}
}




Original code:

static void
writetup_rawheap(Tuplesortstate *state, int tapenum, SortTuple *stup)
{
HeapTuple	tuple = (HeapTuple) stup->tuple;
tuple->t_len += HEAPTUPLESIZE; /* write out the header as well */

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

if (state->randomAccess)	/* need trailing length word? */
 	 LogicalTapeWrite(state->tapeset, tapenum,
 					   tuple, sizeof(tuple->t_len));

FREEMEM(state, GetMemoryChunkSpace(tuple));
heap_freetuple(tuple);
}

static void
readtup_rawheap(Tuplesortstate *state, SortTuple *stup,
int tapenum, unsigned int tuplen)
{
HeapTuple	tuple = (HeapTuple) palloc(tuplen);

USEMEM(state, GetMemoryChunkSpace(tuple));

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");
if (state->randomAccess)	/* need trailing length word? */
if (LogicalTapeRead(state->tapeset, tapenum, &tuplen,
sizeof(tuplen)) != sizeof(tuplen))
elog(ERROR, "unexpected end of data");

stup->tuple = tuple;
/* set up first-column key value */
stup->datum1 = heap_getattr(tuple,
state->scanKeys[0].sk_attno,
state->tupDesc,
&stup->isnull1);
}



      

In response to

Responses

pgsql-hackers by date

Next:From: Fujii MasaoDate: 2010-02-10 09:19:01
Subject: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
Previous:From: Kurt HarrimanDate: 2010-02-10 09:00:52
Subject: Re: Patch: Remove gcc dependency in definition of inline functions

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