Re: Something's been bugging me

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

> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>> I'm wondering whether it doesn't make sense to lower VARATT_SHORT_MAX to 0x70
>> to allow for at least a small number of constant values which could indicate
>> some special type of datum. That could be used to indicate that a fixed size
>> pointer like a toast pointer follows. That could be used for something like
>> common value compression. [*]
>
> I'm not for this because it would complicate the already-too-complicated
> inner-loop tests for deciding which form of datum you're looking at.
>
> The idea that I recall mentioning was to expend another byte in TOAST
> pointers to make them self-identifying, ie, instead of 0x80 or 0x01
> signaling something that *must* be a 17-byte toast pointer, that bit
> pattern signals "something else" and the content of the next byte
> lets you know what. So TOAST pointers would take 18 bytes instead of
> 17, and there would be room for additions of other sorts of pointers.

Here's a patch that does all of the above.

I accidentally included the debugging ifdef to test with textin producing
short varlenas since that was in my tree but thought I would leave it in since
it's just as easy for you to remove it as me and it might be useful to leave
in since it's in an ifdef anyways.

Two style issues: 1) the magic constant 0 could either be hard coded into
postgres.h for now or should become a VARATT_EXTERNAL_TYPE_POINTER or
something like that. 2) the test in tuptoaster.c could be

if (toast_isnull[i] ||
!VARATT_IS_EXTERNAL(new_value) ||
VARSIZE_EXTERNAL(old_value) != VARSIZE_EXTERNAL(new_value) ||
memcmp(VARDATA_SHORT(old_value),
VARDATA_SHORT(new_value),
VARSIZE_EXTERNAL(old_value)) != 0)

which optimizes away to the same thing but I thought it was clearer to leave
those niceties out for now. tuptoaster.c would probably always have to know
what type of pointers it's looking at at that point anyways.