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

Andres Freund <andres(at)2ndquadrant(dot)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