From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <7BF5468D-51A1-42BE-A4AD-CC8211F52D6E@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_ACFBFF33-3789-4D1E-9EF6-481935DF54F9"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: tablecmds: clarify recurse vs recusing
Date: Thu, 28 May 2026 17:26:55 +0800
In-Reply-To: 
 <CAEoWx2m=dditqdNvwC45HNMmzU0jVvN1_2dsP4ma2Gj4Wu7HhQ@mail.gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
References: 
 <CAEoWx2n9E6_zxPbqwMpaPuC1C_p4b3y635SjiDuCPSVm8GBjsA@mail.gmail.com>
 <ba7cba89-b76f-4d8b-a5cd-8ff1fe67bec4@eisentraut.org>
 <1078495.1768839280@sss.pgh.pa.us>
 <CAEoWx2=QnOBUCuTg4rt7n0FjyvhBfztWMffSdObGCbGrz2tg6w@mail.gmail.com>
 <1734390.1768939652@sss.pgh.pa.us>
 <CAEoWx2m=dditqdNvwC45HNMmzU0jVvN1_2dsP4ma2Gj4Wu7HhQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/7BF5468D-51A1-42BE-A4AD-CC8211F52D6E%40gmail.com>
Precedence: bulk


--Apple-Mail=_ACFBFF33-3789-4D1E-9EF6-481935DF54F9
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Jan 21, 2026, at 06:55, Chao Li <li.evan.chao@gmail.com> wrote:
>=20
>=20
>=20
>> On Jan 21, 2026, at 04:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>=20
>> Chao Li <li.evan.chao@gmail.com> writes:
>>> Enhancing the header comments also helps here.
>>> PSA v2:
>>=20
>> I had something more like the attached in mind.  I'm not generally
>> a fan of documenting only some of the arguments of a function, so
>> I don't care for the way you handled the issue for other functions
>> in tablecmds.c either.  We can either assume that people read
>> ATPrepCmd's comment and can extrapolate to the other functions,
>> or we can do something similar to this for all of them.
>=20
> Got it, thanks. In v4, I=E2=80=99ve limited the changes to fully =
documenting all ATPrepCmd() arguments in its header comment and removed =
the scattered recurse/recursing references from other functions.
>=20
>>=20
>> I do agree with your 0002, but I see no point in pushing that
>> separately.
>=20
> Absolutely. 0002 was too trivial to be a separate commit. I have =
squashed it into 0001 in v4.
>=20
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>=20
>=20
>=20
>=20
> <v4-0001-tablecmds-clarify-recurse-recursing-semantics-in-.patch>

Trying to bump this patch because, while working on patch [1], I needed =
to think about the meaning of =E2=80=9Crecurse=E2=80=9D and =
=E2=80=9Crecursing=E2=80=9D. That reminded me of this patch, and it =
helped me recall the distinction. So I still think this pure comment =
update patch is still helpful.

Attached v5 is just a rebased version, with no changes from v4.

[1] =
https://www.postgresql.org/message-id/E74C57FA-1DD0-4C8E-8FB1-538034752592=
%40gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_ACFBFF33-3789-4D1E-9EF6-481935DF54F9
Content-Disposition: attachment;
	filename=v5-0001-tablecmds-clarify-recurse-recursing-semantics-in-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v5-0001-tablecmds-clarify-recurse-recursing-semantics-in-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=209108d4544a425ae8a1818e3a7dc722d00c65bb59=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Tue,=2020=20Jan=202026=2009:57:24=20+0800=0ASubject:=20[PATCH=20=
v5]=20tablecmds:=20clarify=20recurse/recursing=20semantics=20in=0A=20=
ATPrepCmd()=0A=0ADocument=20all=20ATPrepCmd()=20arguments=20and=20=
centralize=20the=20explanation=20of=20how=0Arecurse=20and=20recursing=20=
interact=20during=20ALTER=20TABLE=20recursion.=20Remove=0Ascattered=20=
partial=20comments=20and=20keep=20other=20function=20comments=20=
lightweight.=0A=0ANo=20functional=20changes.=0A=0AAuthor:=20Chao=20Li=20=
<lic@highgo.com>=0AReviewed-by:=20Tom=20Lane=20<tgl@sss.pgh.pa.us>=0A=
Discussion:=20=
https://postgr.es/m/CAEoWx2n9E6_zxPbqwMpaPuC1C_p4b3y635SjiDuCPSVm8GBjsA@ma=
il.gmail.com=0A---=0A=20src/backend/commands/tablecmds.c=20|=2035=20=
+++++++++++++++++++++++++-------=0A=201=20file=20changed,=2028=20=
insertions(+),=207=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/commands/tablecmds.c=20b/src/backend/commands/tablecmds.c=0A=
index=20a1845240a98..63cd1df71bc=20100644=0A---=20=
a/src/backend/commands/tablecmds.c=0A+++=20=
b/src/backend/commands/tablecmds.c=0A@@=20-4577,11=20+4577,10=20@@=20=
AlterTableLookupRelation(AlterTableStmt=20*stmt,=20LOCKMODE=20lockmode)=0A=
=20=20*=0A=20=20*=20The=20caller=20must=20lock=20the=20relation,=20with=20=
an=20appropriate=20lock=20level=0A=20=20*=20for=20the=20subcommands=20=
requested,=20using=20AlterTableGetLockLevel(stmt->cmds)=0A-=20*=20or=20=
higher.=20We=20pass=20the=20lock=20level=20down=0A-=20*=20so=20that=20we=20=
can=20apply=20it=20recursively=20to=20inherited=20tables.=20Note=20that=20=
the=0A-=20*=20lock=20level=20we=20want=20as=20we=20recurse=20might=20=
well=20be=20higher=20than=20required=20for=0A-=20*=20that=20specific=20=
subcommand.=20So=20we=20pass=20down=20the=20overall=20lock=20=
requirement,=0A-=20*=20rather=20than=20reassess=20it=20at=20lower=20=
levels.=0A+=20*=20or=20higher.=20We=20pass=20the=20lock=20level=20down=20=
so=20that=20we=20can=20apply=20it=20recursively=0A+=20*=20to=20inherited=20=
tables.=20Note=20that=20the=20lock=20level=20we=20want=20as=20we=20=
recurse=20might=0A+=20*=20well=20be=20higher=20than=20required=20for=20=
that=20specific=20subcommand.=20So=20we=20pass=20down=0A+=20*=20the=20=
overall=20lock=20requirement,=20rather=20than=20reassess=20it=20at=20=
lower=20levels.=0A=20=20*=0A=20=20*=20The=20caller=20also=20provides=20a=20=
"context"=20which=20is=20to=20be=20passed=20back=20to=0A=20=20*=20=
utility.c=20when=20we=20need=20to=20execute=20a=20subcommand=20such=20as=20=
CREATE=20INDEX.=0A@@=20-4960,8=20+4959,29=20@@=20=
ATController(AlterTableStmt=20*parsetree,=0A=20=20*=20Traffic=20cop=20=
for=20ALTER=20TABLE=20Phase=201=20operations,=20including=20simple=0A=20=20=
*=20recursion=20and=20permission=20checks.=0A=20=20*=0A-=20*=20Caller=20=
must=20have=20acquired=20appropriate=20lock=20type=20on=20relation=20=
already.=0A-=20*=20This=20lock=20should=20be=20held=20until=20commit.=0A=
+=20*=20*wqueue:=20resulting=20AlteredTableInfo=20structs=20are=20added=20=
to=20this=20list=0A+=20*=20rel:=20the=20relation=20we=20are=20currently=20=
considering=0A+=20*=20cmd:=20the=20ALTER=20TABLE=20subcommand=20we=20are=20=
currently=20considering=0A+=20*=20recurse:=20true=20to=20recurse=20to=20=
child=20tables=20of=20rel=20(including=20partitions)=0A+=20*=20=
recursing:=20true=20if=20already=20recursing=20(rel=20is=20a=20=
descendant=20of=20original=20table)=0A+=20*=20lockmode:=20lock=20level=20=
held=20on=20rel,=20and=20to=20be=20acquired=20on=20children=0A+=20*=20=
context:=20context=20passed=20down=20from=20ProcessUtility,=20or=20NULL=20=
if=20none=0A+=20*=0A+=20*=20At=20top=20level,=20recurse=20is=20true=20=
unless=20the=20command=20specified=20ONLY.=0A+=20*=20Internally,=20=
recurse=20may=20be=20true=20when=20descending=20the=20inheritance=20tree=0A=
+=20*=20on-the-fly,=20or=20false=20when=20the=20tree=20has=20already=20=
been=20expanded=20at=20an=0A+=20*=20outer=20level=20(see=20=
ATSimpleRecursion).=0A+=20*=0A+=20*=20recurse=20and=20recursing=20=
together=20control=20how=20recursion=20proceeds:=0A+=20*=0A+=20*=20=20=20=
recurse=3Dfalse,=20recursing=3Dfalse:=20top-level=20call,=20no=20=
recursion=0A+=20*=20=20=20recurse=3Dtrue,=20=20recursing=3Dfalse:=20=
top-level=20call,=20recurse=20if=20supported=0A+=20*=20=20=20=
recurse=3Dtrue,=20=20recursing=3Dtrue:=20=20recursive=20call,=20continue=20=
recursion=0A+=20*=20=20=20recurse=3Dfalse,=20recursing=3Dtrue:=20=20=
recursive=20call,=20stop=20recursion=0A+=20*=0A+=20*=20Caller=20must=20=
have=20acquired=20lockmode=20on=20rel=20already.=20This=20lock=20should=20=
be=20held=0A+=20*=20until=20commit.=20If=20we=20recurse,=20the=20same=20=
lockmode=20will=20be=20acquired=20on=20child=0A+=20*=20tables.=0A=20=20=
*/=0A=20static=20void=0A=20ATPrepCmd(List=20**wqueue,=20Relation=20rel,=20=
AlterTableCmd=20*cmd,=0A@@=20-10819,6=20+10839,7=20@@=20=
validateFkOnDeleteSetColumns(int=20numfks,=20const=20int16=20*fkattnums,=0A=
=20=20*=20=20=20=20=20=20(...)=20clause=0A=20=20*=20fkdelsetcols:=20the=20=
attnum=20array=20of=20the=20columns=20in=20the=20ON=20DELETE=20SET=0A=20=20=
*=20=20=20=20=20=20NULL/DEFAULT=20clause=0A+=20*=20is_internal:=20true=20=
if=20this=20is=20an=20internally=20generated=20constraint=0A=20=20*=20=
with_period:=20true=20if=20this=20is=20a=20temporal=20FK=0A=20=20*/=0A=20=
static=20ObjectAddress=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_ACFBFF33-3789-4D1E-9EF6-481935DF54F9--