MIME-Version: 1.0
References: <tencent_149BDBB2851AA69104BEAC8D84566722F005@qq.com>
In-Reply-To: <tencent_149BDBB2851AA69104BEAC8D84566722F005@qq.com>
From: Aleksander Alekseev <aleksander@tigerdata.com>
Date: Tue, 26 May 2026 12:16:50 +0300
Message-ID: 
 <CAJ7c6TM1sktewDAcHTt19bNEN8w0+q02Okjc1t8eXcKVT9yE9w@mail.gmail.com>
Subject: Re: [PATCH] Little refactoring of portalcmds.c
To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Cc: Zizhuan Liu <44973863@qq.com>, quanzongliang <quanzongliang@yeah.net>,
	qiuwenhuifx <qiuwenhuifx@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJ7c6TM1sktewDAcHTt19bNEN8w0%2Bq02Okjc1t8eXcKVT9yE9w%40mail.gmail.com>
Precedence: bulk

Hi,

> I have reviewed patch 6113 on CommitFest.
> CommitFest page: https://commitfest.postgresql.org/patch/6113/
>
> The newly introduced check_cursor_name() implements the logic to reject e=
mpty cursor names, which matches the existing checks in PerformCursorOpen()=
 and PerformPortalFetch().
>
> However, this helper function is not a good fit for PerformPortalClose().=
 At the start of this function, there is a check for name =3D=3D NULL =E2=
=80=94 a NULL name corresponds to the CLOSE ALL command =E2=80=94 and this =
check runs prior to validating whether the name is an empty string.
>
> While calling check_cursor_name() inside PerformPortalClose() would keep =
the current behavior intact, it could confuse future readers. This approach=
 also fails to achieve truly consistent cursor name validation across all t=
hree functions, and may add extra maintenance burdens in the long term.
>
> On the other hand, if we only refactor PerformCursorOpen() and PerformPor=
talFetch(), the change would end up being rather minimal.
>
> Considering all these factors, I'd suggest leaving the code as it is.
>
> (Sorry, I tried but still cannot post this to the original thread.)

No problem, in fact from what I can tell you did everything right.

OK, let's withdraw the patch. Thanks for reviewing!

--=20
Best regards,
Aleksander Alekseev