public inbox for [email protected]  

help / color / mirror / Atom feed

From: Ashutosh Bapat <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Difference in dump from original and restored database due to NOT NULL constraints on children
Date: Wed, 27 Nov 2024 18:21:59 +0530
Message-ID: <CAExHW5u+TotftfLH7v_7vT36WvQfYs5Uha7h+6s2htmfrs2_Qg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAExHW5s5xLsWovHukbtRHMYADaFp8328+-sRfBFV3E+mC_-7gQ@mail.gmail.com>
	<[email protected]>

On Wed, Nov 27, 2024 at 5:56 PM Alvaro Herrera <[email protected]> wrote:
>
> 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 noticed that. But two reasons why I chose the backend changes
1. The comment where we add explicit ADD CONSTRAINT is
/*
* Dump additional per-column properties that we can't handle in the
* main CREATE TABLE command.
*/
... snip

/*
* If we didn't dump the column definition explicitly above, and
* it is not-null and did not inherit that property from a parent,
* we have to mark it separately.
*/
if (!shouldPrintColumn(dopt, tbinfo, j) &&
tbinfo->notnull_constrs[j] != NULL &&
(tbinfo->notnull_islocal[j] && !tbinfo->ispartition && !dopt->binary_upgrade))
... snip

The comment seems to say that we can not handle the NOT NULL
constraint property in the CREATE TABLE command. Don't know why. We
add CHECK constraints separately in CREATE TABLE even if we didn't add
corresponding columns in CREATE TABLE. So there must be a reason not
to dump NOT NULL constraints that way and hence we required separate
code like above. I am afraid going that direction will show us some
other problems.

2. Name of the constraint provided by ALTER TABLE ... ADD CONSTRAINT
... being silently ignored didn't seem right to me. Especially when we
were adjusting the constraint to be local.

-- 
Best Wishes,
Ashutosh Bapat

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Difference in dump from original and restored database due to NOT NULL constraints on children
  In-Reply-To: <CAExHW5u+TotftfLH7v_7vT36WvQfYs5Uha7h+6s2htmfrs2_Qg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox