Re: remove ATTRIBUTE_FIXED_PART_SIZE

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: remove ATTRIBUTE_FIXED_PART_SIZE
Date: 2018-08-24 15:47:43
Message-ID: 14453.1535125663@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2018-08-24 15:50:38 Re: Flexible configuration for full-text search
Previous Message Andres Freund 2018-08-24 15:46:23 Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)