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

Andres Freund <andres@2ndquadrant.com> writes:
> On 2015-02-17 05:51:22 +0900, Michael Paquier wrote:
>> diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
>> index e01e6aa..d8789a5 100644
>> --- a/src/include/catalog/pg_authid.h
>> +++ b/src/include/catalog/pg_authid.h
>> @@ -56,8 +56,10 @@ CATALOG(pg_authid,1260) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2842) BKI_SCHEMA_MAC
>> int32        rolconnlimit;    /* max connections allowed (-1=no limit) */
>> 
>> /* remaining fields may be null; use heap_getattr to read them! */
>> -    text        rolpassword;    /* password, if any */
>> timestamptz rolvaliduntil;    /* password expiration time, if any */
>> +#ifdef CATALOG_VARLEN
>> +    text        rolpassword;    /* password, if any */
>> +#endif
>> } FormData_pg_authid;

> That change IIRC is wrong, because it'll make rolvaliduntil until NOT
> NULL (any column that's fixed width and has only fixed with columns
> before it is marked as such).

You were muttering about a BKI_FORCE_NOT_NULL option ... for symmetry,
maybe we could add BKI_FORCE_NULL as well, and then use that for cases
like this?  Also, if we want to insist that these fields be accessed
through heap_getattr, I'd be inclined to put them inside the "#ifdef
CATALOG_VARLEN" to enforce that.

I'm generally -1 on reordering any catalog columns as part of this patch.
There should be zero user-visible change from it IMO.  However, if we
stick both those columns inside the ifdef, we don't need to reorder.
        regards, tom lane