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

On Wed, Nov 27, 2024 at 7:04 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> 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.
>

Ok. Here's the patch implementing the same. As you said, it's a much
simpler patch. The test being developed in [1] passes with this
change. pg_dump and pg_upgrade test suites also pass.

[1] https://www.postgresql.org/message-id/flat/CAExHW5uvx2LEyrUBdctV5gS25Zeb%2B-eXESkK93siQxWSjYFy6A%40mail.gmail.com#c8ed57b77d2f6132d5b8e1ecb2a8c47b

Adding this to CF for CI run.

--
Best Wishes,
Ashutosh Bapat