Re: CLUSTER can change t_len

On 09.11.2010 11:11, Itagaki Takahiro wrote:
> On Tue, Nov 9, 2010 at 12:44 PM, Jeff Davis<pgsql(at)j-davis(dot)com> wrote:
>> See case below. After the item length gets changed, then when reading
>> the tuple later you get a t_len that includes padding.
>
> We can easily find it with pageinspect:
>
> \i pageinspect.sql
> create table foo(i int4);
> insert into foo values(1);
> SELECT lp, lp_len FROM heap_page_items(get_raw_page('foo', 0));
> lp | lp_len
> ----+--------
> 1 | 28
> VACUUM FULL foo;
> SELECT lp, lp_len FROM heap_page_items(get_raw_page('foo', 0));
> lp | lp_len
> ----+--------
> 1 | 32
>
>> We should document in a comment that t_len can mean multiple things. Or,
>> we should fix raw_heap_insert() to be consistent with the rest of the
>> code, which doesn't MAXALIGN the t_len.
>
> We have a comment /* be conservative */ in the function, but I'm not sure
> we actually need the MAXALIGN. However, there would be almost no benefits
> to keep t_len in small value because we often treat memory in MAXALIGN unit.

Hmm, the conservatism at that point affects the free space calculations.
I'm not sure if it makes any difference in practice, but I'm also not
sure it doesn't. pd_upper is always MAXALIGNed, but pd_lower is not.

This would be more in line with what the main heap_insert code does:

--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -641,7 +641,7 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
}

/* And now we can insert the tuple into the page */
- newoff = PageAddItem(page, (Item) heaptup->t_data, len,
+ newoff = PageAddItem(page, (Item) heaptup->t_data, heaptup->t_len,
InvalidOffsetNumber, false, true);
if (newoff == InvalidOffsetNumber)
elog(ERROR, "failed to add tuple");

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com