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

On Thu, Nov 14, 2024 at 6:08 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> Hello Ashutosh,
>
> On 2024-Nov-14, Ashutosh Bapat wrote:
>
> > Because of the test I am developing under discussion at [1], I noticed
> > that the DDLs dumped for inheritance children with NOT NULL
> > constraints defer between original database and database restored from
> > a dump of original database. I investigated those differences, but can
> > not decide whether they are expected and whether they are intentional
> > or not. Look something related to
> > 14e87ffa5c543b5f30ead7413084c25f7735039f.
>
> It does and it is, thanks.
>
> So there are two differences:
>
> 1.
>
> CREATE TABLE generated_stored_tests.gtest1 (
> a integer NOT NULL,
> b integer GENERATED ALWAYS AS ((a * 2)) STORED
> );
>
> CREATE TABLE generated_stored_tests.gtestxx_4 (
> - a integer,
> - b integer NOT NULL
> + a integer NOT NULL,
> + b integer
> )
> INHERITS (generated_stored_tests.gtest1);
>
> In this case, I think it's wrong to add NOT NULL to gtestxx_4.a because
> the constraint was not local originally and it should be acquired by the
> inheritance.

The constaint is local and does not get acquired by the inheritance.
But the DDL to create inheritance child with NOT NULL constraint has
changed, which required a change in adjustment code in that test. The
diff does not indicate any bug in
14e87ffa5c543b5f30ead7413084c25f7735039f. Let's leave that aside.

> I'm not sure about gtestxx_4.b ... the GENERATED
> constraint should induce a not-null if I recall correctly, so an
> explicit not-null marking in the child might not be necessary. But I'll
> have a look.

You may forget that since the NOT NULL constraint was attached to
gtestxx_4.b by the dump adjustment code in that test. It's not there
in the dump itself.

>
> 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.

I created the tables using the same DDLs that the test uses
#CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
#CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS
(notnull_tbl4);

Notice that coninhcount = 1
#\x
Expanded display is on.
db1(at)624204=#select * from pg_constraint where conrelid =
'notnull_tbl4_cld2'::regclass;
-[ RECORD 1 ]--+-----------------------------
oid | 16437
conname | notnull_tbl4_cld2_a_not_null
... snip ...
condeferrable | f
condeferred | f
... snip
conislocal | t
coninhcount | 1
connoinherit | f

Since PRIMARY KEY Is declared in both, parent and child, the
constraint on the child is considered local as well as inherited. In
order to adjust that, it dumps the NOT NULL constraint separately? Is
that so?

`````` original database dump
-- Name: notnull_tbl4_cld2; Type: TABLE; Schema: public; Owner: ashutoshpg
--

CREATE TABLE public.notnull_tbl4_cld2 (
)
INHERITS (public.notnull_tbl4);
ALTER TABLE ONLY public.notnull_tbl4_cld2 ALTER COLUMN a SET NOT NULL;
``````` original database dump ends

```` restored database dump
--
-- Name: notnull_tbl4_cld2; Type: TABLE; Schema: public; Owner: ashutoshpg
--

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;
````` restored database dump ends

The question why is the constraint dumped as ALTER TABLE ... ALTER
COLUMN ... NOT NULL from original database and as an ADD CONSTRAINT
from the restored database? Notice the change in the names of
constraint. On the original database the name is
notnull_tbl4_cld2_a_not_null whereas on the restored database it is
notnull_tbl4_a_not_null. Is that the reason behind the difference in
the DDLs? Looks like a bug to me.

>
> 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 think both of these problems are the same. In both the cases, the
name of the constraint on the restored database is
notnull_tbl4_a_not_null, which is the name of the constraint on the
parent table. I think somewhere the code is ignoring the name of the
constraint on the child table if the constraint is local and also
inherited.

--
Best Wishes,
Ashutosh Bapat