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 empty cursor names, which matches the
existingchecks in PerformCursorOpen() and PerformPortalFetch().
>
> However, this helper function is not a good fit for PerformPortalClose(). At the start of this function, there is a
checkfor name == NULL — a NULL name corresponds to the CLOSE ALL command — and this check runs prior to validating
whetherthe name is an empty string.
>
> While calling check_cursor_name() inside PerformPortalClose() would keep the current behavior intact, it could
confusefuture readers. This approach also fails to achieve truly consistent cursor name validation across all three
functions,and may add extra maintenance burdens in the long term.
>
> On the other hand, if we only refactor PerformCursorOpen() and PerformPortalFetch(), the change would end up being
ratherminimal.
>
> 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!
--
Best regards,
Aleksander Alekseev
