From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Zhihong Yu <zyu@yugabyte.com>
Cc: David G. Johnston <david.g.johnston@gmail.com>
Cc: Daniel Verite <daniel@manitou-mail.org>
Cc: Matthieu Garrigues <matthieu.garrigues@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Dave Cramer <davecramer@postgres.rocks>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Craig Ringer <craig@2ndquadrant.com>
Cc: Vaishnavi Prabakaran <vaishnaviprabakaran@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Claudio Freire <klaussfreire@gmail.com>
Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Subject: Re: PATCH: Batch/pipelining support for libpq
Date: Fri, 19 Feb 2021 15:43:23 -0300
Message-ID: <20210219184323.GA5038@alvherre.pgsql> (raw)
In-Reply-To: <CALNJ-vRrGUW=Uj0b5ZtdV_6=pV6Ge0pxBvfLrYxjvdhPPdKZmg@mail.gmail.com>
On 2021-Jan-21, Zhihong Yu wrote:
> It seems '\\gset or \\aset is not ' would correspond to the check more
> closely.
>
> + if (my_command->argc != 1)
> + syntax_error(source, lineno, my_command->first_line,
> my_command->argv[0],
>
> It is possible that my_command->argc == 0 (where my_command->argv[0]
> shouldn't be accessed) ?
No -- process_backslash_command reads the word and sets it as argv[0].
> + appendPQExpBufferStr(&conn->errorMessage,
> + libpq_gettext("cannot queue commands
> during COPY\n"));
> + return false;
> + break;
>
> Is the break necessary ? Similar comment for pqBatchProcessQueue().
Not necessary. I removed them.
> +int
> +PQexitBatchMode(PGconn *conn)
>
> Since either 0 or 1 is returned, maybe change the return type to bool.
I was kinda tempted to do that, but in reality a lot of libpq's API is
defined like that -- to return 0 (failure)/1 (success) as ints, not
bools. For example see PQsendQuery(). I'm not inclined to do different
for these new functions. (One curious case is PQsetvalue, which returns
int, and is documented as "returns zero for failure" and then uses
"return false").
> Also, the function operates on PGconn - should the function be static
> (pqBatchProcessQueue is) ?
I don't understand this suggestion. How would an application call it,
if we make it static?
Thanks
--
Álvaro Herrera Valdivia, Chile
"Cómo ponemos nuestros dedos en la arcilla del otro. Eso es la amistad; jugar
al alfarero y ver qué formas se pueden sacar del otro" (C. Halloway en
La Feria de las Tinieblas, R. Bradbury)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, zyu@yugabyte.com, david.g.johnston@gmail.com, daniel@manitou-mail.org, matthieu.garrigues@gmail.com, michael@paquier.xyz, davecramer@postgres.rocks, craig@2ndquadrant.com, vaishnaviprabakaran@gmail.com, andres@anarazel.de, michael.paquier@gmail.com, klaussfreire@gmail.com, heikki.linnakangas@iki.fi
Subject: Re: PATCH: Batch/pipelining support for libpq
In-Reply-To: <20210219184323.GA5038@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox