Re: [HACKERS] PATCH: Batch/pipelining support for libpq - Mailing list pgsql-hackers
| From | Vaishnavi Prabakaran |
|---|---|
| Subject | Re: [HACKERS] PATCH: Batch/pipelining support for libpq |
| Date | |
| Msg-id | CAOoUkxTs+Sa-04Mt7-hhwAXot7ZWdMZ9Eg1DPTAzP=7BQw_2mQ@mail.gmail.com Whole thread Raw |
| In response to | Re: [HACKERS] PATCH: Batch/pipelining support for libpq ("Daniel Verite" <daniel@manitou-mail.org>) |
| Responses |
Re: PATCH: Batch/pipelining support for libpq
("Daniel Verite" <daniel@manitou-mail.org>)
Re: PATCH: Batch/pipelining support for libpq (David Steele <david@pgmasters.net>) Re: PATCH: Batch/pipelining support for libpq ("Daniel Verite" <daniel@manitou-mail.org>) |
| List | pgsql-hackers |
On Fri, Mar 17, 2017 at 12:37 AM, Daniel Verite <daniel@manitou-mail.org> wrote:
Vaishnavi Prabakaran wrote:
> So, attached the alternative fix for this issue.
> Please share me your thoughts.
I assume you prefer the alternative fix because it's simpler.
I would like add one more reason for this fix, I think "PQsetSingleRowMode" should be called only when the result is ready to be processed and before starting to consume result as it is documented currently as follows -
"To enter single-row mode, call
PQsetSingleRowMode immediately after a successful call of PQsendQuery (or a sibling function). This mode selection is effective only for the currently executing query. Then call PQgetResult repeatedly..."I agree that first fix (you shared) will allow user to set single-row mode after PQsendQuery, but it also allows user to set this mode at any time of batch processing(not necessarily "immediately after PQsendQuery"), also "mode selection is effective only for the currently executing query" will be false. Please note that I don't see any problem with this deviation. I like to outline that documentation here anyways needs an update/note.
Before going further, I would like to mention that I have modified the documentation of batch processing( in v6 code patch) as below:
"To enter single-row mode, call
PQsetSingleRowMode immediately after a successful call of PQbatchQueueProcess. This mode selection is effective only for the currently executing query. For more information on the use of PQsetSingleRowMode , refer to Section 33.6, “Retrieving Query Results Row-By-Row”. "Please let me know if you think this is not enough but wanted to update section 33.6 also?
> I would also like to hear Craig's opinion on it before applying this fix
> to the original patch, just to make sure am not missing anything here.
+1
The main question is whether the predicates enforced
by PQsetSingleRowMode() apply in batch mode in all cases
when it's legit to call that function. Two predicates
that may be problematic are:
if (conn->asyncStatus != PGASYNC_BUSY)
return 0;
and
if (conn->result)
return 0;
The general case with batch mode is that, from the doc:
"The client interleaves result processing with sending batch queries"
While sending batch queries in middle of result processing, only the query is appended to the list of queries maintained for batch processing and no current connection attribute impacting result processing will be changed. So, calling the PQsetSingleRowMode in-between result processing will fail as it tries to set single-row mode for currently executing query for which result processing is already started.
Note that I've not even tested that here,
I've tested
batching a bunch of queries in a first step and getting the results
in a second step.
I am not confident that the above predicates will be true
in all cases.
Also your alternative fix assumes that we add
a user-visible exception to PQsetSingleRowMode in batch mode,
whereby it must not be called as currently documented:
"call PQsetSingleRowMode immediately after a successful call of
PQsendQuery (or a sibling function)"
My gut feeling is that it's not the right direction, I prefer making
the single-row a per-query attribute internally and keep
PQsetSingleRowMode's contract unchanged from the
user's perspective.
Am going to include the test which you shared in the test patch. Please let me know if you want to cover anymore specific cases to gain confidence.
Thanks & Regards,
Vaishnavi,
Fujitsu Australia.
pgsql-hackers by date: