Feedback-ID: ia2694551:Fastmail
Date: Tue, 15 Apr 2025 23:10:26 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
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
Message-ID: <202504152110.gr3ga55h55q6@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1719550.1744746674@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/202504152110.gr3ga55h55q6%40alvherre.pgsql>
Precedence: bulk

On 2025-Apr-15, Tom Lane wrote:

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

Right.

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

Ah yes, I forgot to set got_children when reading the children list.
This happens within the loop for columns, so the idea is to obtain that
list just once instead of once per column.  I don't think there's any
ill effect from doing it multiple times, but it's wasted work and that's
what led me to adding got_children.  I'll add the assignment.

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

I'll review tomorrow morning, maybe I can find some improvements for
them.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La conclusión que podemos sacar de esos estudios es que
no podemos sacar ninguna conclusión de ellos" (Tanenbaum)