On 09.11.2010 11:11, Itagaki Takahiro wrote:
On Tue, Nov 9, 2010 at 12:44 PM, Jeff Davis<pg...@j-davis.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

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

Reply via email to