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 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 == NULL — a NULL name > corresponds to the CLOSE ALL command — 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 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 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! -- Best regards, Aleksander Alekseev
