Prev: [HACKERS] Bug? Concurrent COMMENT ON and DROP object
Next: Bug? Concurrent COMMENT ON and DROP object
From: Takahiro Itagaki on 6 Jul 2010 03:52

Leonardo F <m_lists(a)yahoo.it> wrote:

> Attached the updated patch (should solve a bug) and a script.

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.

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".

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

From: Takahiro Itagaki on 7 Jul 2010 04:39

Leonardo F <m_lists(a)yahoo.it> wrote:

> I saw that you also changed the writing:
(snip)
> Are we sure it's 100% equivalent?

I think writetup_rawheap() and readtup_rawheap() are a little complex,
but should work as long as there are no padding between t_len and t_self
in HeapTupleData struct.

- It might be cleaner if you write the total item length
and tuple data separately.
- "(char *) tuple + sizeof(tuplen)" might be more robust
than "&tuple->t_self".

Here is a sample code. writetup() and readtup() will be alike.

BTW, we could have LogicalTapeReadExact() as an alias of
LogicalTapeRead() and checking the result because we have
many duplicated codes for "unexpected end of data" errors.


static void
writetup_rawheap(Tuplesortstate *state, int tapenum, SortTuple *stup)
{
HeapTuple tuple = (HeapTuple) stup->tuple;
int tuplen = tuple->t_len + HEAPTUPLESIZE;

LogicalTapeWrite(state->tapeset, tapenum,
&tuplen, sizeof(tuplen));
LogicalTapeWrite(state->tapeset, tapenum,
(char *) tuple + sizeof(tuplen),
HEAPTUPLESIZE - sizeof(tuplen);
LogicalTapeWrite(state->tapeset, tapenum, tuple->t_data, tuple->t_len);
if (state->randomAccess) /* need trailing length word? */
LogicalTapeWrite(state->tapeset, tapenum, &tuplen, sizeof(tuplen));

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,
(char *) tuple + sizeof(tuplen),
HEAPTUPLESIZE - sizeof(tuplen)) != HEAPTUPLESIZE - sizeof(tuplen))
elog(ERROR, "unexpected end of data");
tuple->t_data = (HeapTupleHeader) ((char *) tuple + HEAPTUPLESIZE);
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");


Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Alvaro Herrera on 7 Jul 2010 11:41
Excerpts from Takahiro Itagaki's message of mié jul 07 04:39:38 -0400 2010:

> BTW, we could have LogicalTapeReadExact() as an alias of
> LogicalTapeRead() and checking the result because we have
> many duplicated codes for "unexpected end of data" errors.

I'd just add a boolean "exact required" to the existing functions.

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 | 
Pages: 1
Prev: [HACKERS] Bug? Concurrent COMMENT ON and DROP object
Next: Bug? Concurrent COMMENT ON and DROP object