Re: remove ATTRIBUTE_FIXED_PART_SIZE - Mailing list pgsql-hackers

From Tom Lane
Subject Re: remove ATTRIBUTE_FIXED_PART_SIZE
Date
Msg-id 14453.1535125663@sss.pgh.pa.us
Whole thread Raw
In response to Re: remove ATTRIBUTE_FIXED_PART_SIZE  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: remove ATTRIBUTE_FIXED_PART_SIZE
List pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 21/08/2018 17:38, Peter Eisentraut wrote:
>> On 20/08/2018 15:14, Tom Lane wrote:
>>> I agree this is all moot as long as there's no pad bytes.  What I'm
>>> trying to figure out is if we need to put in place some provisions
>>> to prevent there from being pad bytes at the end of any catalog struct.
>>> According to what Andres is saying, it seems like we do (at least for
>>> ones with varlena fields).

>> Yes, I think there could be a problem.  I took a brief look through the
>> catalogs, and while there are plenty of catalogs with trailing padding,
>> finding that in combination with trailing varlena fields that might
>> legitimately be all null in practice might require a closer look.

> Looking into this a bit more, a few catalogs could use some
> BKI_FORCE_NOT_NULL settings, which then avoids the described situation.
> See attached patch.

Um ... this would be enough to document that we don't think there's a
*read* hazard, but Andres was claiming that there's also a *write* hazard.
That is, if you store into (say) a bool in the last declared column,
the compiler will think it's free to trash what it believes are padding
bytes at the end of the struct.  This is problematic: if there's a
short-header varlena there, the overwrite clobbers data directly; and even
if the varlena is aligned, our tuple traversal code requires the alignment
pad bytes to be zero.

I think what we might need to do, in places where the end of struct is
not already aligned, is to do something like

     bool        is_instead;

#ifdef CATALOG_VARLEN            /* variable-length fields start here */
    pg_node_tree ev_qual;
    pg_node_tree ev_action;
#else
    uint8       dummy;
#endif

and then teach the Catalog.pm routines to ignore the "dummy" fields while
emitting the .bki data.  However, I'd want some automatic verification
that we'd added dummy fields where and only where needed.

BTW, I have no objection to documenting should-not-be-null varlena
fields as you suggest.  But it's cosmetic and doesn't really fix
the problem at hand.

            regards, tom lane


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Next
From: Alexander Korotkov
Date:
Subject: Re: Flexible configuration for full-text search