From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: Tender Wang <tndrwang@gmail.com>, jian he <jian.universality@gmail.com>,
        Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: not null constraints, again
In-reply-to: <202504151911.mezh7j3bizh5@alvherre.pgsql>
References: <202504151911.mezh7j3bizh5@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@alvh.no-ip.org>
	message dated "Tue, 15 Apr 2025 21:11:49 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1719549.1744746674.1@sss.pgh.pa.us>
Date: Tue, 15 Apr 2025 15:51:14 -0400
Message-ID: <1719550.1744746674@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/1719550.1744746674%40sss.pgh.pa.us>
Precedence: bulk

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> However, I've also been looking at this and realized that this code can
> have different structure which may allows us to skip the
> find_inheritance_children() altogether.  The reason is that we already
> scan the parent's list of columns searching for not-null constraints on
> each of them; we only need to run this verification on children for
> columns where there is none in the parent, and then only in the case
> where recursion is turned off.

+1.  Fundamentally the problem here is that pg_restore needs

ALTER TABLE ONLY foo ADD PRIMARY KEY

to not recurse to child tables at all.  It is expecting this command
to acquire a lock on foo and nothing else; and it has already taken
care of making foo's PK column(s) NOT NULL, so there is no reason we
should have to examine the children.

Looking at the patch itself, it doesn't seem like the got_children
flag is accomplishing anything; I guess that was leftover from an
earlier version?  You could declare "List *children" inside the
block where it's used, too.  Basically, this patch is just moving
the check-the-children logic from one place to another.

Also I find the comments still a bit confusing, but maybe that's
on me.

			regards, tom lane