Re: BUG #19074: pg_dump from v18 loses the NOT NULL flag in the inherited table field when dumping v17-databases

On Mon, Oct 6, 2025 at 11:52 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Oct 6, 2025 at 9:29 AM David Rowley <dgrowleyml@gmail.com> wrote:
> >
> > On Mon, 6 Oct 2025 at 16:42, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > I tried to reproduce this, but here is what I see[1] when I dump in
> > > REL_18_STABLE, I noticed that "NOT NULL a" for inherited tables is
> > > included along with the create table statement itself, so this doesn't
> > > seems like an issue, am I missing something?
> >
> > I think so. I believe the intent was to convey the server version is
> > 17.6 and the pg_dump version is 18.0, in which case, there does appear
> > to be a problem.
> >
>
> Yeah you are right, while checking the git log, it seems it got broken
> in this commit [1].  So I just tested on the previous commit and it
> was working fine but got broken on this commit.  Adding Alvaro as he
> has committed this.
>
> [1]
> commit 14e87ffa5c543b5f30ead7413084c25f7735039f
> Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
> Date:   Fri Nov 8 13:28:48 2024 +0100
>     Add pg_constraint rows for not-null constraints

While quickly checking this commit, it seems the problem is that
before this commit we had a simple logic to add an additional ALTER
TABLE to SET NULL, if the attribute has a not null constraint but the
the parent from which it is inherited doesn't have not null constraint
[1].  Whereas after this commit, it seems we removed this ALTER
command and tried to add the not null constraint while creating the
inherited table itself, which is fine.  But here the logic to identify
whether the constraint is local or not for v17 is not correct [2], it
sets "notnull_islocal", only if the "attislocal" is true, which seems
to be wrong.  Because the test case given in this bug the attribute is
not local but it is marked not null.

[1]
if (!shouldPrintColumn(dopt, tbinfo, j) &&
tbinfo->notnull[j] && !tbinfo->inhNotNull[j])
appendPQExpBuffer(q,
"ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n",
foreign, qualrelname,
fmtId(tbinfo->attnames[j]));

[2]
if (fout->remoteVersion >= 180000)
appendPQExpBufferStr(q,
"co.conname AS notnull_name,\n"
"CASE WHEN co.convalidated THEN pt.description"
" ELSE NULL END AS notnull_comment,\n"
"CASE WHEN NOT co.convalidated THEN co.oid "
"ELSE NULL END AS notnull_invalidoid,\n"
"co.connoinherit AS notnull_noinherit,\n"
"co.conislocal AS notnull_islocal,\n");
else
appendPQExpBufferStr(q,
"CASE WHEN a.attnotnull THEN '' ELSE NULL END AS notnull_name,\n"
"NULL AS notnull_comment,\n"
"NULL AS notnull_invalidoid,\n"
"false AS notnull_noinherit,\n"
"a.attislocal AS notnull_islocal,\n");  -- this seems to be problamatic


-- 
Regards,
Dilip Kumar
Google