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

On Thu, Nov 28, 2024 at 4:44 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> 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.

CF entry: https://commitfest.postgresql.org/51/5408/

--
Best Wishes,
Ashutosh Bapat