Re: Difference in dump from original and restored database due to NOT NULL constraints on children

Hi Alvaro,

On Thu, Nov 14, 2024 at 6:08 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

>
> 2.
>
> 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;
> +ALTER TABLE ONLY public.notnull_tbl4_cld2 ADD CONSTRAINT notnull_tbl4_a_not_null NOT NULL a;
>
> CREATE TABLE public.notnull_tbl4_cld3 (
> )
> INHERITS (public.notnull_tbl4);
> -ALTER TABLE ONLY public.notnull_tbl4_cld3 ADD CONSTRAINT a_nn NOT NULL a;
> +ALTER TABLE ONLY public.notnull_tbl4_cld3 ADD CONSTRAINT notnull_tbl4_a_not_null NOT NULL a;
>
>
> For notnull_tbl4_cld2 we try to set the column not-null, but it's
> already not-null due to inheritance ... so why are we doing that?
> Weird. These lines should just go away in both dumps.
>
> In notnull_tbl4_cld3's case we have the same, but we also want to set a
> constraint name, but the constraint already exists, so that doesn't
> work, and that's the reason why the next dump uses the standard name.
> Maybe we could dump the name change as ALTER TABLE RENAME CONSTRAINT in
> that case instead, _if_ we can obtain the original name (should be
> doable, because we can see it's a nonstandard name.)

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

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;Dump and restore of inherited and local child NOT NULL
constraints

Fixes two issues described below:
1. A child table will inherit NOT NULL constraints from the parent. But it's
allowed to add local NOT NULL constraints on the same column. If a user
specifies a name while adding such a constraint it will be ignored. That may not
be intended. But ignoring the given name has an ill-effect on dump and restore
of such a constraint. A named NOT NULL constraint which is inherited and is
also local will be dumped as ALTER TABLE ... ADD CONSTRAINT ... command
specifying its name. If we ignore this name, it will not be restored and thus
lost. Hence while ADDing a local NOT NULL constraint on the child
table, change the
name of the inherited constraint, if one exists.

2. Consider a parent with NOT NULL col1. If a child is created with
CREATE TABLE child (NOT NULL col1) INHERITS (parent), col1 would be an
inherited column with a local as well as inherited NOT NULL constraint
with the default name containing the name of the child table in it.
We will dump "CREATE TABLE child() INHERITS parent" to create the
child table. This will add the inherited constraint on child with name
of the parent constraint. To turn it into a local constraint we will
dump "ALTER TABLE child ALTER COLUMN col1 SET NOT NULL". This will not
restore the original name of the constraint. Hence instead of dumping
ALTER TABLE ... ALTER COLUMN, we dump ALTER TABLE ... ADD CONSTRAINT
with the child's default name. This would preserve the child's default
name across dump and restore.
Dump and restore of inherited and local child NOT NULL constraints

Fixes two issues described below:
1. A child table will inherit NOT NULL constraints from the parent. But it's
allowed to add local NOT NULL constraints on the same column. If a user
specifies a name while adding such a constraint it will be ignored. That may not
be intended. But ignoring the given name has an ill-effect on dump and restore
of such a constraint. A named NOT NULL constraint which is inherited and is
also local will be dumped as ALTER TABLE ... ADD CONSTRAINT ... command
specifying its name. If we ignore this name, it will not be restored and thus
lost. Hence while ADDing a local NOT NULL constraint on the child
table, change the
name of the inherited constraint, if one exists.

2. Consider a parent with NOT NULL col1. If a child is created with
CREATE TABLE child (NOT NULL col1) INHERITS (parent), col1 would be an
inherited column with a local as well as inherited NOT NULL constraint
with the default name containing the name of the child table in it.
We will dump "CREATE TABLE child() INHERITS parent" to create the
child table. This will add the inherited constraint on child with name
of the parent constraint. To turn it into a local constraint we will
dump "ALTER TABLE child ALTER COLUMN col1 SET NOT NULL". This will not
restore the original name of the constraint. Hence instead of dumping
ALTER TABLE ... ALTER COLUMN, we dump ALTER TABLE ... ADD CONSTRAINT
with the child's default name. This would preserve the child's default
name across dump and restore.

... primary key constraint DDLs follow - they don't affect NOT NULL
constraints now

The extra ALTER TABLE ... ALTER COLUMN is because the constraint is
local and has the default name as per logic in
determineNotNullFlags().
When the dump is restored, the status of constraints on notnull_tbl4_cld2 is
#select conname, coninhcount, conislocal, contype from pg_constraint
where conrelid = 'notnull_tbl4_cld2'::regclass;
conname | coninhcount | conislocal | contype
-------------------------+-------------+------------+---------
notnull_tbl4_a_not_null | 1 | t | n
notnull_tbl4_cld2_pkey | 0 | t | p
(2 rows)

Please note that the coninhcount and conislocal are restored, but the
name of the constraint has changed. The name of the constraint is the
same as the parent's constraint name (notnull_tbl4_a_not_null) which
is the default name for parent.
This happens because the CREATE TABLE ... INHERITS () creates an
inherited constraint on the child with the same name of the parent.
ALTER TABLE ... ALTER COLUMN ... SET NOT NULL, changes itsDump and
restore of inherited and local child NOT NULL constraints

Fixes two issues described below:
1. A child table will inherit NOT NULL constraints from the parent. But it's
allowed to add local NOT NULL constraints on the same column. If a user
specifies a name while adding such a constraint it will be ignored. That may not
be intended. But ignoring the given name has an ill-effect on dump and restore
of such a constraint. A named NOT NULL constraint which is inherited and is
also local will be dumped as ALTER TABLE ... ADD CONSTRAINT ... command
specifying its name. If we ignore this name, it will not be restored and thus
lost. Hence while ADDing a local NOT NULL constraint on the child
table, change the
name of the inherited constraint, if one exists.

2. Consider a parent with NOT NULL col1. If a child is created with
CREATE TABLE child (NOT NULL col1) INHERITS (parent), col1 would be an
inherited column with a local as well as inherited NOT NULL constraint
with the default name containing the name of the child table in it.
We will dump "CREATE TABLE child() INHERITS parent" to create the
child table. This will add the inherited constraint on child with name
of the parent constraint. To turn it into a local constraint we will
dump "ALTER TABLE child ALTER COLUMN col1 SET NOT NULL". This will not
restore the original name of the constraint. Hence instead of dumping
ALTER TABLE ... ALTER COLUMN, we dump ALTER TABLE ... ADD CONSTRAINT
with the child's default name. This would preserve the child's default
name across dump and restore.
conislocal to "true' (again because of AdjustNotNullInheritance())
keeping coninhcount same. When the restored database is dumped it
looks like below

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_a_not_null NOT NULL a;
... primary key constraint DDLs follow - they don't affect NOT NULL
constraints now

ALTER TABLE now uses ADD CONSTRAINT since the name of the constraint
is not default one per determineNotNullFlags(). But the original name
of the constraint is lost forever.

If we create a NOT NULL constraint instead of primary key constraint,
we see similar phenomena.
#CREATE TABLE notnull_tbl4 (a INTEGER NOT NULL);
#CREATE TABLE notnull_tbl4_cld2 (NOT NULL a) 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
(1 row)

Dump contains
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;

Restoring this dump
#select conname, coninhcount, conislocal, contype from pg_constraint
where conrelid = 'notnull_tbl4_cld2'::regclass;
conname | coninhcount | conislocal | contype
-------------------------+-------------+------------+---------
notnull_tbl4_a_not_null | 1 | t | n
(1 row)
Notice the change in the name of the constraint.

Second case: Named NOT NULL constraint
----------------------------------------------------------
On the original database
#CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
#CREATE TABLE notnull_tbl4_cld3 (PRIMARY KEY (a) DEFERRABLE,
CONSTRAINT a_nn NOT NULL a) INHERITS (notnull_tbl4);
#select conname, coninhcount, conislocal, contype from pg_constraint
where conrelid = 'notnull_tbl4_cld3'::regclass;
conname | coninhcount | conislocal | contype
------------------------+-------------+------------+---------
a_nn | 1 | t | n
notnull_tbl4_cld3_pkey | 0 | t | p
(2 rows)
Upto this the only difference in the previous case and this case is
the name of the constraint - a_nn which is user specified.

The dump contains
CREATE TABLE public.notnull_tbl4 (
a integer NOT NULL
);
CREATE TABLE public.notnull_tbl4_cld3 (
)
INHERITS (public.notnull_tbl4);
ALTER TABLE ONLY public.notnull_tbl4_cld3 ADD CONSTRAINT a_nn NOT NULL a;

The constraint is dumped separately since it's local; same as the
first case. However, it uses ADD CONSTAINT instead of ALTER COLUMN
since the name of the constraint is not default.
When this dump is restored, the status of the constraint is
#select conname, coninhcount, conislocal, contype from pg_constraint
where conrelid = 'notnull_tbl4_cld3'::regclass;
conname | coninhcount | conislocal | contype
-------------------------+-------------+------------+---------
notnull_tbl4_a_not_null | 1 | t | n
notnull_tbl4_cld3_pkey | 0 | t | p
(2 rows)

Notice that the name of the constraint has not been restored since the
logic in AdjustNotNullInheritance() does not change the name of the
inherited constraint while changing conislocal.
Rest of the story is the same as the first case.

From this analysis, it looks like we need to add ADD CONSTRAINT or
ALTER COLUMN ... SET NOT NULL ... in order to mark the NOT NULL
constraint as local.
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(at)gmx(dot)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.

Instead I chose to fix both the problems by
1. In AdjustNotNullInheritance(), if a NOT NULL constraint is
converted to local and the user has specified a name for it (ALTER
TABLE ... ADD CONSTRAINT ... NOT NULL ..), change the name alongwith
setting conislocal. The user who specified a non-default name in ADD
CONSTRAINT would expect the "local" constraint to be named
accordingly. If a local constraint is ADDed again with a different
name, right now the patch ignores the new name but that could be
changed to throw an error similar to ALTER TABLE ... RENAME CONSTAINT.
2. In determineNotNullFlags(), if a NOT NULL constraint is both
inherited and local, preserve its name even if it's default. That way,
it will be dumped as ALTER TABLE ... ADD CONSTRAINT ... NOT NULL ...
instead of ALTER TABLE ... ALTER COLUMN ... SET NOT NULL. An exception
to this is a local NOT NULL column with default name for NOT NULL
constraint. Such NOT NULL constraints are specified in the CREATE
TABLE ... INHERITS .. command itself. That way they preserve their
default name.

The first change will preserve the given name of the child constraint.
Both changes together will preserve the default name of the child
constraint.
See the patch. I have also added some tests.

Alternate approach which I haven't tried, but did consider and left aside.
1. Allow RENAME CONSTAINT on a local constaint even if it's inherited.
Given that the behaviour is 13 years old, I am hesitant to change it.
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.

--
Best Wishes,
Ashutosh Bapat