I'm proposing some minor changes.
--
Justin
>From 35ed4dc1fc770834972396f7eeed142f6dabee88 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 3 Mar 2021 16:36:40 -0600
Subject: [PATCH 2/2] doc review
---
doc/src/sgml/libpq.sgml | 67 +++++++++++++-------------
src/interfaces/libpq/fe-exec.c | 2 +-
src/test/modules/test_libpq/pipeline.c | 6 +--
3 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index a34ecbb144..e2865a218b 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -3130,8 +3130,9 @@ ExecStatusType PQresultStatus(const PGresult *res);
<term><literal>PGRES_PIPELINE_SYNC</literal></term>
<listitem>
<para>
- The <structname>PGresult</structname> represents a point in a
- pipeline where a synchronization point has been established.
+ The <structname>PGresult</structname> represents a
+ synchronization point in pipeline mode, requested by
+ <xref linkend="libpq-PQsendPipeline"/>.
This status occurs only when pipeline mode has been selected.
</para>
</listitem>
@@ -3141,9 +3142,9 @@ ExecStatusType PQresultStatus(const PGresult *res);
<term><literal>PGRES_PIPELINE_ABORTED</literal></term>
<listitem>
<para>
- The <structname>PGresult</structname> represents a pipeline that's
+ The <structname>PGresult</structname> represents a pipeline that has
received an error from the server. <function>PQgetResult</function>
- must be called repeatedly, and it will return this status code,
+ must be called repeatedly, and each time it will return this status code
until the end of the current pipeline, at which point it will return
<literal>PGRES_PIPELINE_SYNC</literal> and normal processing can
resume.
@@ -4953,7 +4954,7 @@ int PQflush(PGconn *conn);
<title>Using Pipeline Mode</title>
<para>
- To issue pipelines the application must switch a connection into pipeline mode.
+ To issue pipelines, the application must switch a connection into pipeline mode.
Enter pipeline mode with <xref linkend="libpq-PQenterPipelineMode"/>
or test whether pipeline mode is active with
<xref linkend="libpq-PQpipelineStatus"/>.
@@ -4975,7 +4976,7 @@ int PQflush(PGconn *conn);
server will block trying to send results to the client from queries
it has already processed. This only occurs when the client sends
enough queries to fill both its output buffer and the server's receive
- buffer before switching to processing input from the server,
+ buffer before it switches to processing input from the server,
but it's hard to predict exactly when that will happen.
</para>
</footnote>
@@ -5025,7 +5026,7 @@ int PQflush(PGconn *conn);
<title>Processing Results</title>
<para>
- To process the result of one pipeline query, the application calls
+ To process the result of one query in a pipeline, the application calls
<function>PQgetResult</function> repeatedly and handles each result
until <function>PQgetResult</function> returns null.
The result from the next query in the pipeline may then be retrieved using
@@ -5057,10 +5058,10 @@ int PQflush(PGconn *conn);
<type>PGresult</type> types <literal>PGRES_PIPELINE_SYNC</literal>
and <literal>PGRES_PIPELINE_ABORTED</literal>.
<literal>PGRES_PIPELINE_SYNC</literal> is reported exactly once for each
- <function>PQsendPipeline</function> call at the corresponding point in
- the result stream.
+ <function>PQsendPipeline</function> after retrieving results for all
+ queries in the pipeline.
<literal>PGRES_PIPELINE_ABORTED</literal> is emitted in place of a normal
- result stream result for the first error and all subsequent results
+ stream result for the first error and all subsequent results
except <literal>PGRES_PIPELINE_SYNC</literal> and null;
see <xref linkend="libpq-pipeline-errors"/>.
</para>
@@ -5075,7 +5076,8 @@ int PQflush(PGconn *conn);
application about the query currently being processed (except that
<function>PQgetResult</function> returns null to indicate that we start
returning the results of next query). The application must keep track
- of the order in which it sent queries and the expected results.
+ of the order in which it sent queries, to associate them with their
+ corresponding results.
Applications will typically use a state machine or a FIFO queue for this.
</para>
@@ -5091,10 +5093,10 @@ int PQflush(PGconn *conn);
</para>
<para>
- From the client perspective, after the client gets a
- <literal>PGRES_FATAL_ERROR</literal> return from
- <function>PQresultStatus</function> the pipeline is flagged as aborted.
- <application>libpq</application> will report
+ From the client perspective, after <function>PQresultStatus</function>
+ returns <literal>PGRES_FATAL_ERROR</literal>,
+ the pipeline is flagged as aborted.
+ <function>PQresultStatus</function>, will report a
<literal>PGRES_PIPELINE_ABORTED</literal> result for each remaining queued
operation in an aborted pipeline. The result for
<function>PQsendPipeline</function> is reported as
@@ -5108,8 +5110,8 @@ int PQflush(PGconn *conn);
</para>
<para>
- If the pipeline used an implicit transaction then operations that have
- already executed are rolled back and operations that were queued for after
+ If the pipeline used an implicit transaction, then operations that have
+ already executed are rolled back and operations that were queued to follow
the failed operation are skipped entirely. The same behaviour holds if the
pipeline starts and commits a single explicit transaction (i.e. the first
statement is <literal>BEGIN</literal> and the last is
@@ -5145,11 +5147,11 @@ int PQflush(PGconn *conn);
<para>
The client application should generally maintain a queue of work
- still to be dispatched and a queue of work that has been dispatched
+ remaining to be dispatched and a queue of work that has been dispatched
but not yet had its results processed. When the socket is writable
it should dispatch more work. When the socket is readable it should
read results and process them, matching them up to the next entry in
- its expected results queue. Based on available memory, results from
+ its expected results queue. Based on available memory, results from the
socket should be read frequently: there's no need to wait until the
pipeline end to read the results. Pipelines should be scoped to logical
units of work, usually (but not necessarily) one transaction per pipeline.
@@ -5191,8 +5193,8 @@ int PQflush(PGconn *conn);
<listitem>
<para>
- Returns current pipeline mode status of the <application>libpq</application>
- connection.
+ Returns the current pipeline mode status of the
+ <application>libpq</application> connection.
<synopsis>
PGpipelineStatus PQpipelineStatus(const PGconn *conn);
</synopsis>
@@ -5233,11 +5235,10 @@ PGpipelineStatus PQpipelineStatus(const PGconn *conn);
<listitem>
<para>
The <application>libpq</application> connection is in pipeline
- mode and an error has occurred while processing the current
- pipeline.
- The aborted flag is cleared as soon as the result
- of the <function>PQsendPipeline</function> at the end of the aborted
- pipeline is processed. Clients don't usually need this function to
+ mode and an error occurred while processing the current pipeline.
+ The aborted flag is cleared when <function>PQresultStatus</function>
+ returns PGRES_PIPELINE_SYNC at the end of the pipeline.
+ Clients don't usually need this function to
verify aborted status, as they can tell that the pipeline is aborted
from the <literal>PGRES_PIPELINE_ABORTED</literal> result code.
</para>
@@ -5328,7 +5329,7 @@ int PQsendPipeline(PGconn *conn);
Returns 1 for success. Returns 0 if the connection is not in
pipeline mode or sending a
<link linkend="protocol-flow-ext-query">sync message</link>
- is failed.
+ failed.
</para>
</listitem>
</varlistentry>
@@ -5349,7 +5350,7 @@ int PQsendPipeline(PGconn *conn);
<para>
Pipeline mode is most useful when the server is distant, i.e., network latency
(<quote>ping time</quote>) is high, and also when many small operations
- are being performed in rapid sequence. There is usually less benefit
+ are being performed in rapid succession. There is usually less benefit
in using pipelined commands when each query takes many multiples of the client/server
round-trip time to execute. A 100-statement operation run on a server
300ms round-trip-time away would take 30 seconds in network latency alone
@@ -5367,7 +5368,7 @@ int PQsendPipeline(PGconn *conn);
<para>
Pipeline mode is not useful when information from one operation is required by
the client to produce the next operation. In such cases, the client
- must introduce a synchronization point and wait for a full client/server
+ would have to introduce a synchronization point and wait for a full client/server
round-trip to get the results it needs. However, it's often possible to
adjust the client design to exchange the required information server-side.
Read-modify-write cycles are especially good candidates; for example:
@@ -5392,10 +5393,10 @@ UPDATE mytable SET x = x + 1 WHERE id = 42;
<note>
<para>
- The pipeline API was introduced in PostgreSQL 14, but clients using
- the PostgreSQL 14 version of <application>libpq</application> can use
- pipelines on server versions 7.4 and newer. Pipeline mode works on any server
- that supports the v3 extended query protocol.
+ The pipeline API was introduced in <productname>PostgreSQL</productname> 14.
+ Pipeline mode is a client-side feature which doesn't require server
+ support, and works on any server that supports the v3 extended query
+ protocol.
</para>
</note>
</sect2>
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 7ae7c14948..d7b036a35c 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3803,7 +3803,7 @@ pqPipelineFlush(PGconn *conn)
{
if ((conn->pipelineStatus == PQ_PIPELINE_OFF) ||
(conn->outCount >= OUTBUFFER_THRESHOLD))
- return (pqFlush(conn));
+ return pqFlush(conn);
return 0;
}
diff --git a/src/test/modules/test_libpq/pipeline.c b/src/test/modules/test_libpq/pipeline.c
index f4a2bdec57..01d5a9a8ff 100644
--- a/src/test/modules/test_libpq/pipeline.c
+++ b/src/test/modules/test_libpq/pipeline.c
@@ -126,7 +126,7 @@ test_simple_pipeline(PGconn *conn)
* process the results of as they come in.
*
* For a simple case we should be able to do this without interim
- * processing of results since our out buffer will give us enough slush to
+ * processing of results since our output buffer will give us enough slush to
* work with and we won't block on sending. So blocking mode is fine.
*/
if (PQisnonblocking(conn))
@@ -603,7 +603,7 @@ test_pipelined_insert(PGconn *conn, int n_rows)
/*
* Now we start inserting. We'll be sending enough data that we could fill
- * our out buffer, so to avoid deadlocking we need to enter nonblocking
+ * our output buffer, so to avoid deadlocking we need to enter nonblocking
* mode and consume input while we send more output. As results of each
* query are processed we should pop them to allow processing of the next
* query. There's no need to finish the pipeline before processing
@@ -635,7 +635,7 @@ test_pipelined_insert(PGconn *conn, int n_rows)
}
/*
- * Process any results, so we keep the server's out buffer free
+ * Process any results, so we keep the server's output buffer free
* flowing and it can continue to process input
*/
if (FD_ISSET(sock, &input_mask))
--
2.17.0
