public inbox for [email protected]  

help / color / mirror / Atom feed

From: Alvaro Herrera <[email protected]>
To: Ashutosh Bapat <[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 14:33:58 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAExHW5u+TotftfLH7v_7vT36WvQfYs5Uha7h+6s2htmfrs2_Qg@mail.gmail.com>

On 2024-Nov-27, Ashutosh Bapat wrote:

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

I don't think this is an important restriction.  We can change that, as
long as all cases work correctly.  We previously didn't try to use
"CONSTRAINT foobar NOT NULL a" because 1) we didn't support the
table-constraint syntax for not-null constraint and 2) not-null
constraint didn't support names anyway.  We now support that syntax, so
we can use it.

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

We could make this throw an error when the names don't match; I don't
think that'd affect anything important.  I'm not in love with the idea
of ADD CONSTRAINT changing the constraint name.  Or, to be more precise,
I think having ALTER TABLE ADD CONSTRAINT change the name of an existing
constraint is a terrible idea.  I will forever be shamed publicly if I
let that happen, and frankly I don't need any more reasons to be shamed
publicly, as I have plenty already.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Use it up, wear it out, make it do, or do without"

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: <[email protected]>

* 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