Re: pg17 issues with not-null contraints

On 2024-Apr-15, Alvaro Herrera wrote:

> On 2024-Apr-15, Justin Pryzby wrote:

> > 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);

> Hmm, apparently if the table is "iparent", the primary key is created in
> the child first; if the table is "parent", then the PK is created first
> there.  I think the problem is that the ADD CONSTRAINT for the PK should
> not be recursing at all in this case ... seeing in particular that the
> command specifies ONLY.  Should be a simple fix, looking now.

So the problem is that the ADD CONSTRAINT PRIMARY KEY in the parent
table wants to recurse to the child, so that a NOT NULL constraint is
created on each column.  If the child is created first, there's already
a NOT NULL NO INHERIT constraint in it which was created for its own
primary key, so the internal recursion in the parent's ADD PK fails.

A fix doesn't look all that simple:

- As I said in my earlier reply, my first thought was to have ALTER
TABLE ADD PRIMARY KEY not recurse if the command is ALTER TABLE ONLY.
This doesn't work, because the point of that recursion is precisely to
handle this case, so if we do that, we break the other stuff that this
was added to solve.

- Second thought was to add a bespoke dependency in pg_dump.c so that
the child PK is dumped after the parent PK.  I looked at the code,
didn't like the idea of adding such a hack, went looking for other
ideas.

- Third thought was to hack AdjustNotNullInheritance1() so that it
changes the conisnoinherit flag in this particular case.  Works great,
except that once we mark this constraint as inherited, we cannot drop
it; and since it's a constraint marked "throwaway", pg_dump expects to
be able to drop it, which means the ALTER TABLE DROP CONSTRAINT throws
an error, and a constraint named pgdump_throwaway_notnull_0 remains in
place.

- Fourth thought: we do as in the third thought, except we also allow
DROP CONSTRAINT a constraint that's marked "local, inherited" to be
simply an inherited constraint (remove its "local" marker).

I'm going to try to implement this fourth idea, which seems promising.
I think if we do that, the end result will be identical to the case
where the child is created after the parent.

However, we'll also need that constraint to have a name better than
pgdump_throwaway_notnull_NN.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/