Feedback-ID: ia2694551:Fastmail
Date: Thu, 20 Mar 2025 11:26:26 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Rushabh Lathia <rushabh.lathia@gmail.com>
Cc: jian he <jian.universality@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Support NOT VALID / VALIDATE constraint options for named NOT
 NULL constraints
Message-ID: <202503201026.6lzmnbhxfovj@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAGPqQf1x2xMG2d19bqyFDEPTni7h=8T-Gi+_oLRGGGAMzy9tZQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202503201026.6lzmnbhxfovj%40alvherre.pgsql>
Precedence: bulk

Hello

On 2025-Mar-20, Rushabh Lathia wrote:

> Attached is another version of the patch (WIP), where I have
> introduced a new catalog column, pg_attribute.attinvalidnotnull
> (boolean). This column will default to FALSE but will be set to TRUE
> when an INVALID NOT NULL constraint is created.  With this approach,
> we can avoid performing extra scans on the catalog table to identify
> INVALID NOT NULL constraints, ensuring there is no performance impact.

I find this a reasonable way forward.  We won't impact any applications
that are currently relying on the boolean semantics of attnotnull, while
still allowing the backend to know the real status of the constraint
without having to scan pg_constraint for it.

I also checked the struct layout, which is currently

struct FormData_pg_attribute {
    Oid                        attrelid;             /*     0     4 */
    NameData                   attname;              /*     4    64 */
    /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
    Oid                        atttypid;             /*    68     4 */
    int16                      attlen;               /*    72     2 */
    int16                      attnum;               /*    74     2 */
    int32                      atttypmod;            /*    76     4 */
    int16                      attndims;             /*    80     2 */
    _Bool                      attbyval;             /*    82     1 */
    char                       attalign;             /*    83     1 */
    char                       attstorage;           /*    84     1 */
    char                       attcompression;       /*    85     1 */
    _Bool                      attnotnull;           /*    86     1 */
    _Bool                      atthasdef;            /*    87     1 */
    _Bool                      atthasmissing;        /*    88     1 */
    char                       attidentity;          /*    89     1 */
    char                       attgenerated;         /*    90     1 */
    _Bool                      attisdropped;         /*    91     1 */
    _Bool                      attislocal;           /*    92     1 */

    /* XXX 1 byte hole, try to pack */

    int16                      attinhcount;          /*    94     2 */
    Oid                        attcollation;         /*    96     4 */

    /* size: 100, cachelines: 2, members: 20 */
    /* sum members: 99, holes: 1, sum holes: 1 */
    /* last cacheline: 36 bytes */
};

Since there's a one-byte hole after the lot of bools/chars, the new bool
will use it and this won't enlarge the storage, neither in memory nor on
disk.

I have not reviewed this patch in detail yet.

Thank you!

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/