Two aesthetic bugs in the 1-byte packed varlena code

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Two aesthetic bugs in the 1-byte packed varlena code
Date: 2007-06-12 11:34:17
Message-ID: 87645th0ee.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


Korry spotted two violations of the rule about accessing the first varlena
field directly which I had missed. In both cases I believe the accesses are
actually safe because they follow an int4 column and the data are either
explicitly detoasted or passed to textout which can handle 1-byte headers
fine.

One instance is in flatfiles.c when it accesses the password field which is a
text field. Here I don't see any reasonable way to avoid it and think we
should just document that we're bending the rules.

The other instance is in inv_api.c where it would be quite possible to use
fastgetattr() instead. But the column is always at the same fixed offset and
again it follows an int4 so it'll always be 4-byte aligned and work fine. So
for performance reasons perhaps we should keep this as well?

I've attached three patches. A patch which adds a comment for flatfiles.c. And
two patches for inv_api.c one which just adds a comment and one which converts
it to use fastgetattr().

Again, credit to Korry for spotting these. To do so he commented out all the
varlena fields (excluding int2vector and oidvector) from the struct
definitions and saw what broke.

Why do we even have those fields in the structs if they're unsafe to use?
Perhaps we should #if 0 out all the unsafe fields from all the struct
definitions? That would avoid one of the most common new programmer bugs and
also let us document the few exceptions and why they're allowed.

Attachment Content-Type Size
inv_api.c-minimal.gz application/octet-stream 1.0 KB
flatfiles.c-minimal.gz application/octet-stream 687 bytes
inv_api.c-fastgetattr.gz application/octet-stream 603 bytes

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Michael Meskes 2007-06-12 12:25:24 Re: Regression tests
Previous Message Zdenek Kotala 2007-06-12 11:20:21 Re: Autovacuum launcher doesn't notice death of postmaster immediately