Andres Freund <and...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to