Re: CLUSTER can change t_len

On 09.11.2010 17:14, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> On 09.11.2010 11:11, Itagaki Takahiro wrote:
>>> 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.
>
> I tend to agree with Jeff's original point that the behavior should
> match regular tuple insertion exactly. This isn't about saving space,
> because it won't; it's about not confusing readers by doing the same
> thing in randomly different ways. I will also note that the regular
> path is FAR better tested than raw_heap_insert. If there are any bugs
> here, it's 1000:1 they're in raw_heap_insert not the regular path.

Agreed. I've committed my patch to make it behave like heap_insert.
Thank you, Itagaki, for the easy test case using pageinspect.

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