Re: Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
Date: 2015-02-18 13:09:18
Message-ID: 20150218130918.GA16383@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015-02-18 17:15:18 +0900, Michael Paquier wrote:
> >> - I don't think that the t_bits fields in htup_details.h should be
> >> updated either.
> >
> > Why not? Any not broken code should already use MinHeapTupleSize and
> > similar macros.
>
> Changing t_bits impacts HeapTupleHeaderData, ReorderBufferTupleBuf and
> similarly a couple of redo routines in heapam.c using
> HeapTupleHeaderData in a couple of structures not placing it at the
> end (compiler complains).

The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the
middle of a struct but not when when you embed a struct that uses it
into the middle another struct. At least gcc doesn't and I think it'd be
utterly broken if another compiler did that. If there's a compiler that
does so, we need to make it define FLEXIBLE_ARRAY_MEMBER to 1.

Code embedding structs using *either* [FLEXIBLE_ARRAY_MEMBER] or [1] for
variable length obviously has to take care where the variable length
part goes. And that what the structs you removed where doing - that's
where the t_bits et al go:
{
...
HeapTupleHeaderData header;
char data[MaxHeapTupleSize];
...
}
the 'data' bit is just the t_bits + data together. And similar in
- struct
- {
- struct varlena hdr;
- char data[TOAST_MAX_CHUNK_SIZE]; /* make struct big enough */
- int32 align_it; /* ensure struct is aligned well enough */
- } chunk_data;

The 'data' is where the varlena's vl_dat stuff is stored.
> >> Places using a variable-length variable not at the end of a structure
> >> are changed with workaround, without impacting what those features do.
> >
> > I vote for rejecting most of this, except a (corrected version) of the
> > pg_authid change. Which doesn't really belong to the rest of the patch
> > anyway ;)x
>
> Changing bytea to use FLEXIBLE_ARRAY_MEMBER requires those changes, or
> at least some changes in this area as something with
> FLEXIBLE_ARRAY_MEMBER can only be placed at the end of a structure.

Again, I think you're confusing things that may not be be done in a
single struct, and structs that are embedded in other places.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-02-18 13:14:00 Re: Parallel Seq Scan
Previous Message Neil Tiffin 2015-02-18 12:59:05 Re: pgaudit - an auditing extension for PostgreSQL