Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx

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

In response to

Responses

Browse pgsql-hackers by date

  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