From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx |
Date: | 2024-07-24 05:04:05 |
Message-ID: | ZqCLRa5OxPqzqWh6@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v6-0001-psql-Add-support-for-prepared-stmt-with-extended-.patch | text/x-diff | 20.6 KB |
v6-0002-psql-Refactor-status-of-extended-protocol-command.patch | text/x-diff | 6.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-07-24 05:07:21 | Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx |
Previous Message | Nisha Moond | 2024-07-24 04:50:24 | Re: Conflict detection and logging in logical replication |