Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: vignesh C <vignesh21@gmail.com>, Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>, Jelte Fennema-Nio <postgres@jeltef.nl>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-07-24T05:07:21Z
Lists: pgsql-hackers
Attachments
- v6-0001-psql-Add-support-for-prepared-stmt-with-extended-.patch (text/x-diff) patch v6-0001
- v6-0002-psql-Refactor-status-of-extended-protocol-command.patch (text/x-diff) patch v6-0002
On Fri, Jul 19, 2024 at 03:28:44PM +0200, Tomas Vondra wrote: > OK, if you're already half-way through the review, I'll leave it up to > you. I don't think we need to rush, and I'd have to learn about all the > psql stuff first anyway. It took me a couple of days to get back to it, but attached is what I have finished with. This was mostly OK, except for a few things: - \close was inconsistent with the other two commands, where no argument was treated as the unnamed prepared statement. I think that this should be made consistent with \parse and \bindx, requiring an argument, where '' is the unnamed statement. - The docs did not mention the case of the unnamed statement, so added some notes about that. - Some free() calls were not needed in the command executions, where psql_scan_slash_option() returns NULL. - Tests missing when no argument is provided for the new commands. One last thing I have found really confusing is that this leads to the addition of two more status flags in pset for the close and parse parts, with \bind and \bindx sharing the third one while deciding which path to use depending on if the statement name is provided. That's fragile. I think that it would be much cleaner to put all that behind an enum, falling back to PQsendQuery() by default. I am attaching that as 0002, for clarity, but my plan is to merge both 0001 and 0002 together. -- Michael
Commits
-
psql: Rename meta-command \close to \close_prepared
- fc39b286ad72 18.0 landed
-
psql: Add tests for repeated calls of \bind[_named]
- 91287b5f5da3 18.0 landed
-
psql: Fix memory leak with repeated calls of \bind
- c2fb2f9e200f 16.5 landed
- b0ae6db2088b 17.0 landed
-
psql: Clean up more aggressively state of \bind[_named], \parse and \close
- 87eeadaea143 18.0 landed
-
psql: Add more meta-commands able to use the extended protocol
- d55322b0da60 18.0 landed
-
psql: Add ignore_slash_options in bind's inactive branch
- 04c0897d3bca 17.0 landed