On 2015-02-16 21:34:57 -0500, Tom Lane wrote: > 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?
Yea, I guess it'd not be too hard. > 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. That we definitely should do. It's imo just a small bug that it was omitted here. I'll fix it, but not backpatch unless you prefer? > 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. Agreed. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
