pg17 issues with not-null contraints - Mailing list pgsql-hackers

Forking: <20230829172828.5qi2pfbladvfgvsg@alvherre.pgsql>
Subject: Re: Strange presentaion related to inheritance in \d+

On Tue, Aug 29, 2023 at 07:28:28PM +0200, Alvaro Herrera wrote:
> On 2023-Aug-29, Kyotaro Horiguchi wrote:
> 
> > Attached is the initial version of the patch. It prevents "CREATE
> > TABLE" from executing if there is an inconsisntent not-null
> > constraint.  Also I noticed that "ALTER TABLE t ADD NOT NULL c NO
> > INHERIT" silently ignores the "NO INHERIT" part and fixed it.
> 
> Great, thank you.  I pushed it after modifying it a bit -- instead of
> throwing the error in MergeAttributes, I did it in
> AddRelationNotNullConstraints().  It seems cleaner this way, mostly
> because we already have to match these two constraints there.  (I guess
> you could argue that we waste catalog-insertion work before the error is
> reported and the whole thing is aborted; but I don't think this is a
> serious problem in practice.)

9b581c5341 can break dump/restore from old versions, including
pgupgrade.

postgres=# CREATE TABLE iparent(id serial PRIMARY KEY); CREATE TABLE child (id int) INHERITS (iparent); ALTER TABLE
childALTER id DROP NOT NULL; ALTER TABLE child ADD CONSTRAINT p PRIMARY KEY (id);
 

$ pg_dump -h /tmp -p 5678 postgres -Fc |pg_restore -1 -h /tmp -p 5679 -d postgres
ERROR:  cannot change NO INHERIT status of inherited NOT NULL constraint "pgdump_throwaway_notnull_0" on relation
"child"
STATEMENT:  ALTER TABLE ONLY public.iparent
            ADD CONSTRAINT iparent_pkey PRIMARY KEY (id);
        ALTER TABLE ONLY public.iparent DROP CONSTRAINT pgdump_throwaway_notnull_0;

Strangely, if I name the table "parent", it seems to work, which might
indicate an ordering/dependency issue.

I think there are other issues related to b0e96f3119 (Catalog not-null
constraints) - if I dump a v16 server using v17 tools, the backup can't
be restored into the v16 server.  I'm okay ignoring a line or two like
'unrecognized configuration parameter "transaction_timeout", but not
'syntax error at or near "NO"'.

postgres=# CREATE TABLE a(i int not null primary key);

$ pg_dump -h /tmp -p 5678 postgres |psql -h /tmp -p 5678 -d new

2024-04-13 21:26:14.510 CDT [475995] ERROR:  syntax error at or near "NO" at character 86
2024-04-13 21:26:14.510 CDT [475995] STATEMENT:  CREATE TABLE public.a (
            i integer CONSTRAINT pgdump_throwaway_notnull_0 NOT NULL NO INHERIT
        );
ERROR:  syntax error at or near "NO"
LINE 2: ...er CONSTRAINT pgdump_throwaway_notnull_0 NOT NULL NO INHERIT

The other version checks in pg_dump.c are used to construct sql for
querying the source db, but this is used to create the sql to restore
the target, using syntax that didn't exist until v17.

                    if (print_notnull)  
                    {
                        if (tbinfo->notnull_constrs[j][0] == '\0')
                            appendPQExpBufferStr(q, " NOT NULL");
                        else
                            appendPQExpBuffer(q, " CONSTRAINT %s NOT NULL",
                                              fmtId(tbinfo->notnull_constrs[j]));

                        if (tbinfo->notnull_noinh[j])
                            appendPQExpBufferStr(q, " NO INHERIT");
                    }

This other thread is 6 years old and forgotten again, but still seems
relevant.
https://www.postgresql.org/message-id/flat/b8794d6a-38f0-9d7c-ad4b-e85adf860fc9%40enterprisedb.com

BTW, these comments are out of date:

+        * In versions 16 and up, we need pg_constraint for explicit NOT NULL
+       if (fout->remoteVersion >= 170000)

+ *   that we needn't specify that again for the child. (Versions >= 16 no
+                       if (fout->remoteVersion < 170000)

-- 
Justin