Обсуждение: BUG #19074: pg_dump from v18 loses the NOT NULL flag in the inherited table field when dumping v17-databases

The following bug has been logged on the website:

Bug reference:      19074
Logged by:          Andrew Bille
Email address:      andrewbille@gmail.com
PostgreSQL version: 18.0
Operating system:   Ubuntu 20.04
Description:

Hello.

In 17.6, we're creating tables:

CREATE TABLE p (a integer);
CREATE TABLE c () INHERITS (p);
ALTER TABLE ONLY c ALTER COLUMN a SET NOT NULL;

17/bin/pg_dump test returns:

....
CREATE TABLE public.p (
a integer
);

ALTER TABLE public.p OWNER TO andrew;

--
-- Name: c; Type: TABLE; Schema: public; Owner: andrew
--

CREATE TABLE public.c (
)
INHERITS (public.p);
ALTER TABLE ONLY public.c ALTER COLUMN a SET NOT NULL;
....


REL_18_0, REL_18_STABLE, master
master/bin/pg_dump test produces:

...
CREATE TABLE public.p (
a integer
);


ALTER TABLE public.p OWNER TO andrew;

--
-- Name: c; Type: TABLE; Schema: public; Owner: Andrew
--

CREATE TABLE public.c (
)
INHERITS (public.p);


ALTER TABLE public.c OWNER TO andrew;

--
-- Data for Name: c; Type: TABLE DATA; Schema: public; Owner: Andrew
--

COPY public.c (a) FROM stdin;
\.
...

and loses NOT NULL in the inherited table

Regards, Andrew

On Sun, Oct 5, 2025 at 3:22 PM PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference:      19074
> Logged by:          Andrew Bille
> Email address:      andrewbille@gmail.com
> PostgreSQL version: 18.0
> Operating system:   Ubuntu 20.04
> Description:
>
> Hello.
>
> In 17.6, we're creating tables:
>
> CREATE TABLE p (a integer);
> CREATE TABLE c () INHERITS (p);
> ALTER TABLE ONLY c ALTER COLUMN a SET NOT NULL;
>
> 17/bin/pg_dump test returns:
>
> ....
> CREATE TABLE public.p (
> a integer
> );
>
> ALTER TABLE public.p OWNER TO andrew;
>
> --
> -- Name: c; Type: TABLE; Schema: public; Owner: andrew
> --
>
> CREATE TABLE public.c (
> )
> INHERITS (public.p);
> ALTER TABLE ONLY public.c ALTER COLUMN a SET NOT NULL;
> ....
>
>
> REL_18_0, REL_18_STABLE, master
> master/bin/pg_dump test produces:
>

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?

[1]
CREATE TABLE public.p (
    a integer
);

ALTER TABLE public.p OWNER TO dilipkumarb;

--
-- Name: c; Type: TABLE; Schema: public; Owner: dilipkumarb
--

CREATE TABLE public.c (
    NOT NULL a
)
INHERITS (public.p);

ALTER TABLE public.c OWNER TO dilipkumarb;


--
Regards,
Dilip Kumar
Google

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.

David

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


--
Regards,
Dilip Kumar
Google

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

On 2025-Oct-06, Dilip Kumar wrote:

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

Ah, right, the column is indeed not local, but the constraint is.  I
think this means we need to use flagInhAttrs to require the constraint
to be printed ... looking.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

On Mon, Oct 6, 2025 at 4:14 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-06, Dilip Kumar wrote:
>
> > 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.
>
> Ah, right, the column is indeed not local, but the constraint is.  I
> think this means we need to use flagInhAttrs to require the constraint
> to be printed ... looking.
>

I think we can fix it in getTableAttrs(), see attached, with that I
can see my dump is correct.  This is just a quick way to show what I
am thinking, maybe we can improve this condition.  I believe
flagInhAttrs() is more about resetting the "notnull_islocal" if any of
the parent already has it as non null.  This fix is working with this
basic case, but I haven't investigated whether it will work in all
cases or is it breaking anything.

After this fix the dump look like this

CREATE TABLE public.p (
    a integer
);


ALTER TABLE public.p OWNER TO dilipkumarb;

--
-- Name: c; Type: TABLE; Schema: public; Owner: dilipkumarb
--

CREATE TABLE public.c (
    NOT NULL a        -- now after fix NOT NULL constraint is added in the dump
)
INHERITS (public.p);


--
Regards,
Dilip Kumar
Google

On 2025-Oct-17, Andrew Bille wrote:

> Looks good for me. I checked it on my enhanced upgrade test.

Cool, thanks for checking.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No necesitamos banderas
 No reconocemos fronteras"                  (Jorge González)

By the way, while testing this, I ran into a pg_dump bug.  In the object
sorting algorithm (DOTypeNameCompare), we fail to process objects of
type DO_FK_CONSTRAINT correctly (which is to say: identically to
DO_CONSTRAINT ones), so we fall back to compare by OID, and in debug
builds, the assertion at the bottom of the routine fires.  The fix is
trivial:

diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index a51064f21e3..0aec83bedb2 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -328,7 +328,8 @@ DOTypeNameCompare(const void *p1, const void *p2)
         if (cmpval != 0)
             return cmpval;
     }
-    else if (obj1->objType == DO_CONSTRAINT)
+    else if (obj1->objType == DO_CONSTRAINT ||
+             obj1->objType == DO_FK_CONSTRAINT)
     {
         ConstraintInfo *robj1 = *(ConstraintInfo *const *) p1;
         ConstraintInfo *robj2 = *(ConstraintInfo *const *) p2;


I'll apply this separately, because it goes back to 13 (commit
0decd5e89db9).

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Computing is too important to be left to men." (Karen Spärck Jones)

On 2025-Oct-06, Dilip Kumar wrote:

> I think we can fix it in getTableAttrs(), see attached, with that I
> can see my dump is correct.  This is just a quick way to show what I
> am thinking, maybe we can improve this condition.

Pushed, thanks.  I added a test so that cross-version upgrade will hit
this.  Because this code is not used except when upgrading from 17 or
older, I had to modify 17 even though that version needs no code changes
itself.  (I verified that the pg_upgrade test fails if I have this table
in a pg_dumpall from version 17 -- and the diff is exactly this missing
NOT NULL constraint.)

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Investigación es lo que hago cuando no sé lo que estoy haciendo"
(Wernher von Braun)