Re: not null constraints, again - Mailing list pgsql-hackers

Sadly, there were some other time-wasting events that I failed to
consider, but here's now v3 which has fixed (AFAICS) all the problems
you reported.

On 2024-Sep-11, jian he wrote:

> after applying your changes.
>
> in ATExecAddConstraint, ATAddCheckNNConstraint.
> ATAddCheckNNConstraint(wqueue, tab, rel,
>             newConstraint, recurse, false, is_readd,
>             lockmode);
> if passed to ATAddCheckNNConstraint rel is a partitioned table.
> ATAddCheckNNConstraint itself can recurse to create not-null pg_constraint
> for itself and it's partitions (children table).
> This is fine as long as we only call ATExecAddConstraint once.
>
> but ATExecAddConstraint itself will recurse, it will call
> the partitioned table and each of the partitions.

Yeah, this is because ATPrepAddPrimaryKey was queueing SetNotNull nodes
for each column on each children, which is repetitive and causes the
problem you see.  That was a leftover from the previous way we handled
PKs; we no longer need it to work that way.  I have changed it so that
it queues one constraint addition per column, on the same table that
receives the PK.  It now works correctly as far as I can tell.

Sadly, there's one more pg_dump issue, which causes the pg_upgrade tests
to fail.  The problem is that if you have this sequence (taken from
constraints.sql):

CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS (notnull_tbl4);

this is dumped by pg_dump in this other way:

CREATE TABLE public.notnull_tbl4 (a integer NOT NULL);
CREATE TABLE public.notnull_tbl4_cld2 () INHERITS (public.notnull_tbl4);
ALTER TABLE ONLY public.notnull_tbl4_cld2 ADD CONSTRAINT notnull_tbl4_cld2_pkey PRIMARY KEY (a) DEFERRABLE;
ALTER TABLE ONLY public.notnull_tbl4 ADD CONSTRAINT notnull_tbl4_pkey PRIMARY KEY (a) DEFERRABLE INITIALLY DEFERRED;

This is almost exactly the same, except that the PK for
notnull_tbl4_cld2 is created in a separate command ... and IIUC this
causes the not-null constraint to obtain a different name, or a
different inheritance characteristic, and then from the
restored-by-pg_upgrade database, it's dumped by pg_dump separately.
This is what causes the pg_upgrade test to fail.

Anyway, this made me realize that there is a more general problem, to
wit, that pg_dump is not dumping not-null constraint names correctly --
sometimes they just not dumped, which is Not Good.  I'll have to look
into that once more.


(Also: there are still a few additional test stanzas in regress/ that
ought to be removed; also, I haven't re-tested sepgsql, so it's probably
broken ATM.)

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