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: <202504161124.4ypvhzbnecbm@alvherre.pgsql>
References: <202504161124.4ypvhzbnecbm@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@alvh.no-ip.org>
	message dated "Wed, 16 Apr 2025 13:24:13 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2057834.1744833309.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 16 Apr 2025 15:55:09 -0400
Message-ID: <2057835.1744833309@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2057835.1744833309%40sss.pgh.pa.us>
Precedence: bulk

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Here's another version where I do skip searching for children twice, and
> rewrote the comments.

v2 LGTM, with two small nits:

1. Grammar feels shaky here:

+ * normal case where we're asked to recurse, this routine ensures that th=
e
+ * not-null constraints either exist already, or queues a requirement for=
 them
+ * to be created by phase 2.

The "either" seems to apply to "ensures" versus "queues", but it's in
the wrong place for that.  Maybe something like

+ * normal case where we're asked to recurse, this routine checks if the
+ * not-null constraints exist already, and if not queues a requirement fo=
r
+ * them to be created by phase 2.

2. Stupider compilers are likely to whine about the "children"
variable possibly being used uninitialized.  Suggest initializing
it to NIL.

			regards, tom lane