Обсуждение: [PATCH] Use idiomatic style for varlena structs

I'm new to the codebase, but I think this patch reflects real-world usage;
the PostgreSQL code itself always calls the length field "vl_len_", and I
believe int32 is preferred over int4 (yes?)

Jay Levitt

Jay Levitt <jay.levitt@gmail.com> writes:
> I'm new to the codebase, but I think this patch reflects real-world usage;
> the PostgreSQL code itself always calls the length field "vl_len_", and I
> believe int32 is preferred over int4 (yes?)

The point of calling it vl_len_ is that it should never be referenced by
that name, so I'm not sure that propagating that name into user
documentation is a good idea.  I do agree with the part of this patch
that recommends use of SET_VARSIZE.

For context, the issues you're concerned about only matter when dealing
with a toastable datatype (not all varlena types are toastable).  The
particular bit of docs here doesn't pretend to be explaining how to
write toast-safe code.  I think it might be better from an expository
standpoint to cover that separately, rather than try to work it into the
very first pass over the concepts.

            regards, tom lane

Tom Lane wrote:
> Jay Levitt<jay.levitt@gmail.com>  writes:
>> I'm new to the codebase, but I think this patch reflects real-world usage;
>> the PostgreSQL code itself always calls the length field "vl_len_", and I
>> believe int32 is preferred over int4 (yes?)
>
> The point of calling it vl_len_ is that it should never be referenced by
> that name, so I'm not sure that propagating that name into user
> documentation is a good idea.  I do agree with the part of this patch
> that recommends use of SET_VARSIZE.

Ah, ok.  My confusion came from trying to build a new extension, using cube
as a baseline; cube (and all the other contrib extensions) use vl_len_, so I
saw a disconnect between the "int4 length" in the manual and the
"int32 vl_len_" in all the real-world examples I had.

My thought was that "vl_len_" *feels* more like a "this is mysterious and
I'd better not touch it - I'll use that macro", while "length" feels more
like something I might just set myself if I didn't know better.  But maybe
not.  Either way, the int32/int4 thing should be fixed.

> For context, the issues you're concerned about only matter when dealing
> with a toastable datatype (not all varlena types are toastable).  The
> particular bit of docs here doesn't pretend to be explaining how to
> write toast-safe code.  I think it might be better from an expository
> standpoint to cover that separately, rather than try to work it into the
> very first pass over the concepts.

Definitely; if anything, that's why I was favoring vl_len_. This is a magic
field. Do not touch the magic.

Jay

On Mon, Feb 13, 2012 at 1:57 PM, Jay Levitt <jay.levitt@gmail.com> wrote:
> Tom Lane wrote:
>>
>> Jay Levitt<jay.levitt@gmail.com>  writes:
>>>
>>> I'm new to the codebase, but I think this patch reflects real-world
>>> usage;
>>> the PostgreSQL code itself always calls the length field "vl_len_", and I
>>> believe int32 is preferred over int4 (yes?)
>>
>>
>> The point of calling it vl_len_ is that it should never be referenced by
>> that name, so I'm not sure that propagating that name into user
>> documentation is a good idea.  I do agree with the part of this patch
>> that recommends use of SET_VARSIZE.
>
>
> Ah, ok.  My confusion came from trying to build a new extension, using cube
> as a baseline; cube (and all the other contrib extensions) use vl_len_, so I
> saw a disconnect between the "int4 length" in the manual and the
> "int32 vl_len_" in all the real-world examples I had.
>
> My thought was that "vl_len_" *feels* more like a "this is mysterious and
> I'd better not touch it - I'll use that macro", while "length" feels more
> like something I might just set myself if I didn't know better.  But maybe
> not.  Either way, the int32/int4 thing should be fixed.
>
>
>> For context, the issues you're concerned about only matter when dealing
>> with a toastable datatype (not all varlena types are toastable).  The
>> particular bit of docs here doesn't pretend to be explaining how to
>> write toast-safe code.  I think it might be better from an expository
>> standpoint to cover that separately, rather than try to work it into the
>> very first pass over the concepts.
>
>
> Definitely; if anything, that's why I was favoring vl_len_. This is a magic
> field. Do not touch the magic.

I've committed the parts of this to which Tom did not object.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company