Re: Two aesthetic bugs in the 1-byte packed varlena code

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-patches" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Two aesthetic bugs in the 1-byte packed varlena code
Date: 2007-06-12 17:42:14
Message-ID: 874plddq89.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>> The other instance is in inv_api.c where it would be quite possible to use
>> fastgetattr() instead. But the column is always at the same fixed offset and
>> again it follows an int4 so it'll always be 4-byte aligned and work fine. So
>> for performance reasons perhaps we should keep this as well?
>
> After looking closer, I think there are worse problems here: the code is
> still using VARSIZE/VARDATA etc, which it should not be because the
> field could easily be in 1-byte-header form.

Well that's ok because VARATT_IS_EXTENDED returns true for 1-byte forms so
it'll detoast them first. We could avoid the detoasting but given that it's
expecting the chunks to be compressed anyways the memcpys of the smallest
chunks probably don't matter much either way. I'm assuming it's like toast in
that only the last chunk will be smaller than LOBLKSIZE anyways, right?

> I concur that we probably want to avoid adding fastgetattr for
> performance reasons, seeing that this table's layout seems unlikely
> to change. But it seems like we might want to trouble with a null
> test. Thoughts?

There should never even be a null bitmap right? Maybe we should just
elog(ERROR) if we find HeapTupleHasNulls(tuple) to be true at all.

Gregory Stark
EnterpriseDB http://www.enterprisedb.com

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2007-06-12 19:12:37 Re: Two aesthetic bugs in the 1-byte packed varlena code
Previous Message Greg Smith 2007-06-12 17:28:56 Re: trace_checkpoint parameter patch