> Yes, I have created a new patch entry into the commitfest 2017-03 and > attached the latest patch with this e-mail.
Please find attached a companion patch implementing the batch API in pgbench, exposed as \beginbatch and \endbatch meta-commands (without documentation).
The idea for now is to make it easier to exercise the API and test how batching performs. I guess I'll submit the patch separately in a future CF, depending on when/if the libpq patch goes in.
Thanks for the companion patch and here are some comments:
1. I see, below check is used to verify if the connection is not in batch mode:
if (PQbatchStatus(st->con) != PQBATCH_MODE_ON)
But, it is better to use if (PQbatchStatus(st->con) == PQBATCH_MODE_OFF) for this verification. Reason is there is one more state in batch mode - PQBATCH_MODE_ABORTED. So, if the batch mode is in aborted status, this check will still assume that the connection is not in batch mode.
In a same way, "if(PQbatchStatus(st->con) == PQBATCH_MODE_ON)" check is better to be modified as "PQbatchStatus(st->con) != PQBATCH_MODE_OFF".
This is not required as below PQbatchBegin() internally does this verification.
+PQbatchBegin(st->con);
3. It is better to check the return value of PQbatchBegin() rather than assuming success. E.g: PQbatchBegin() will return false if the connection is in copy in/out/both states.
While developing this, I noted a few things with 0001-v4:
1. lack of initialization for count in PQbatchQueueCount. Trivial fix:
--- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1874,7 +1874,7 @@ PQisBusy(PGconn *conn) int PQbatchQueueCount(PGconn *conn) { - int count; + int count = 0; PGcommandQueueEntry *entry;
Thanks for your review and yes, Corrected.
2. misleading error message in PQexecStart. It gets called by a few other functions than PQexec, such as PQprepare. As I understand it, the intent here is to forbid the synchronous functions in batch mode, so this error message should not single out PQexec.
@@ -1932,6 +2425,13 @@ PQexecStart(PGconn *conn) if (!conn) return false;
Hmm, this error message goes with the flow of other error messages in the same function. E.g: "PQexec not allowed during COPY BOTH" . But, anyways I modified the
error message to be more generic.
3. In relation to #2, PQsendQuery() is not forbidden in batch mode although I don't think it can work with it, as it's based on the old protocol.
It is actually forbidden and you can see the error message "cannot PQsendQuery in batch mode, use PQsendQueryParams" when you use PQsendQuery().
