pg.ddx.io pgsql-docs@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] Use idiomatic style for varlena structs
4+ messages / 3 participants
[nested] [flat]
* [PATCH] Use idiomatic style for varlena structs
@ 2012-02-13 14:45 Jay Levitt <jay.levitt@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Jay Levitt @ 2012-02-13 14:45 UTC (permalink / raw)
To: pgsql-docs
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
Attachments:
vl_len_.patch (text/plain, 1.6K) download
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index a6d2a13..b19801d 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -1733,9 +1733,11 @@ typedef struct
<para>
Finally, all variable-length types must also be passed
by reference. All variable-length types must begin
- with a length field of exactly 4 bytes, and all data to
+ with an opaque length field of exactly 4 bytes, which will be set
+ by SET_VARSIZE; never set this field directly! All data to
be stored within that type must be located in the memory
- immediately following that length field. The
+ immediately following the length field, which is customarily
+ named <literal>vl_len_</literal>. The
length field contains the total length of the structure,
that is, it includes the size of the length field
itself.
@@ -1766,7 +1768,7 @@ typedef struct
<programlisting>
typedef struct {
- int4 length;
+ int32 vl_len_;
char data[1];
} text;
</programlisting>
@@ -1799,10 +1801,10 @@ memcpy(destination->data, buffer, 40);
]]>
</programlisting>
- <literal>VARHDRSZ</> is the same as <literal>sizeof(int4)</>, but
+ <literal>VARHDRSZ</> is the same as <literal>sizeof(int32)</>, but
it's considered good style to use the macro <literal>VARHDRSZ</>
to refer to the size of the overhead for a variable-length type.
- Also, the length field <emphasis>must</> be set using the
+ Also, the vl_len_ field <emphasis>must</> be set using the
<literal>SET_VARSIZE</> macro, not by simple assignment.
</para>
* Re: [PATCH] Use idiomatic style for varlena structs
@ 2012-02-13 15:51 Tom Lane <tgl@sss.pgh.pa.us>
parent: Jay Levitt <jay.levitt@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2012-02-13 15:51 UTC (permalink / raw)
To: Jay Levitt <jay.levitt@gmail.com>; +Cc: pgsql-docs
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Use idiomatic style for varlena structs
@ 2012-02-13 18:57 Jay Levitt <jay.levitt@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Jay Levitt @ 2012-02-13 18:57 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-docs
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Use idiomatic style for varlena structs
@ 2012-03-22 19:49 Robert Haas <robertmhaas@gmail.com>
parent: Jay Levitt <jay.levitt@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: Robert Haas @ 2012-03-22 19:49 UTC (permalink / raw)
To: Jay Levitt <jay.levitt@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-docs
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
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2012-03-22 19:49 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2012-02-13 14:45 [PATCH] Use idiomatic style for varlena structs Jay Levitt <jay.levitt@gmail.com>
2012-02-13 15:51 ` Tom Lane <tgl@sss.pgh.pa.us>
2012-02-13 18:57 ` Jay Levitt <jay.levitt@gmail.com>
2012-03-22 19:49 ` Robert Haas <robertmhaas@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox