Re: Difference in dump from original and restored database due to NOT NULL constraints on children - Mailing list pgsql-hackers

On 2024-Nov-27, Ashutosh Bapat wrote:

> I studied this in more details. Here's what is happening

First, thank you very much for spending time on this!

> First case: unnamed/default named constraint
> -------------------------------------------------------------
> On original database following DDLs are executed
> #CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
> #CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS
> (notnull_tbl4);
> #select conname, coninhcount, conislocal, contype from pg_constraint
> where conrelid = 'notnull_tbl4_cld2'::regclass;
>            conname            | coninhcount | conislocal | contype
> ------------------------------+-------------+------------+---------
>  notnull_tbl4_cld2_a_not_null |           1 | t          | n
>  notnull_tbl4_cld2_pkey       |           0 | t          | p
> (2 rows)
> 
> Though the child inherited primary key constraint it was overridden by
> local constraint that's how we see coninhcount = 0 and conislocal = t.
> But NOT NULL constraint shows both inherited and local (coninhcount =
> 1 and conislocal = t) because of the logic in
> AdjustNotNullInheritance(). When the table is dumped, it is dumped as
> 
> 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 ALTER COLUMN a SET NOT NULL;

Hmm.  Actually, I think this would work fine if we make pg_dump emit the
constraint with its name with the child creation, _without the column_:

CREATE TABLE public.notnull_tbl4_cld2 (                                                                               
   PRIMARY KEY (a) DEFERRABLE,
   CONSTRAINT notnull_tbl4_cld2_a_not_null NOT NULL a
)
INHERITS (public.notnull_tbl4);

This works already seems a lot simpler.  The column definition is
obtained from the parent, but the constraint is defined locally in
addition to inherited.  So we just need to change pg_dump to dump local
constraints using that syntax; no backend changes needed.

I elided a lot of your email here because I didn't understand it.
I think you pasted your commit message several times.

> You suggested using ALTER TABLE ... RENAME CONSTRAINT, but renaming an
> inherited constraint is not allowed.
> #alter table notnull_tbl4_cld3 rename constraint
> notnull_tbl4_a_not_null to a_nn;
> ERROR:  cannot rename inherited constraint "notnull_tbl4_a_not_null"
> ... that goes back 13 years.
> commit 39d74e346c083aa371ba64c4edb1332c40b56530
> Author: Peter Eisentraut <peter_e@gmx.net>
> Date:   Sat Mar 10 20:19:13 2012 +0200
> 
>     Add support for renaming constraints
> 
>     reviewed by Josh Berkus and Dimitri Fontaine
> So I don't think we should change that behaviour.

I don't necessarily agree with feeling ourselves restricted by this; I
have to assume that that case was forbidden for CHECK constraints
because they must have identical names on all children, because we match
them by name when walking down hierarchies for whatever DDL operation.
But as has been established, constraint names don't have to be identical
for not-null constraints, because we don't match them by name.

However, I'm not sure we need to remove this restriction, at least not
to fix this problem.

As far as backwards compatibility arguments go, I think we need to
observe a rule like: if something worked previously, then it should
continue to work, otherwise there's a regression.  But if something
didn't previously work, then it's okay to make it work.  You're not
regressing anything.


> 2. Allow local and inherited NOT NULL constraints to co-exist
> separately (similar to primary key constraint). I am not sure why we
> don't allow this behaviour. AdjustNotNullInheritance() doesn't explain
> the reason.

IIRC we discussed this during the previous discussion of not-null
constraints (in the 16 cycle).  The main problem is that if you have two
not-null constraints for the same column, and the user runs ALTER TABLE
.. DROP NOT NULL, what do you think should happen?  We saw three
alternatives, neither of them good:

1. arbitrarily choose one to drop, leave the others alone
2. remove them all
3. error out and ask the user to use DROP CONSTRAINT instead, to choose
which one to drop.


If you look at the SQL standard 2016 edition, they have 11.16 <drop
column not null clause>, whose General Rules say this:

1) If the column descriptor of C contains an indication that C is defined as
   NOT NULL, then:
   a) Let ACN be the constraint name of the associated table constraint
      definition included in the column descriptor of C.
   b) The column descriptor of C is modified as follows:
      i) The indication that C is defined as NOT NULL is removed.
      ii) The constraint name of the associated table constraint definition is removed.

This is clearly assuming that we have a single constraint to drop.
I don't think it'd be wise to change this.  Initially we allowed
multiple not-null constraints per column, and when we decided to
disallow it, a lot of code that supported that case could be removed.
I'm not inclined to put that back.  Things start to become brittle if
you do; I had to work pretty hard to make it all robust.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"I can see support will not be a problem.  10 out of 10."    (Simon Wittber)
      (http://archives.postgresql.org/pgsql-general/2004-12/msg00159.php)