Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 16 Jan 2024 12:32:22 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>,
	Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>,
	Anton Kirilov <antonvkirilov@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: Add PQsendSyncMessage() to libpq
Message-ID: <ZaX4xtwfSelqP5KQ@paquier.xyz>
References: <ZaTjr6qpw71vS00O@paquier.xyz>
 <202401150949.wq7ynlmqxphy@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="rS2+ijLgHRrNbVax"
Content-Disposition: inline
In-Reply-To: <202401150949.wq7ynlmqxphy@alvherre.pgsql>
Archived-At: <https://www.postgresql.org/message-id/ZaX4xtwfSelqP5KQ%40paquier.xyz>
Precedence: bulk


--rS2+ijLgHRrNbVax
Content-Type: multipart/mixed; boundary="N5Oy+HUAY7ygRgcX"
Content-Disposition: inline


--N5Oy+HUAY7ygRgcX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 15, 2024 at 10:49:56AM +0100, Alvaro Herrera wrote:
> the new function pqPipelineSyncInternal is not a wrapper for these other
> two functions -- the opposite is true actually.  We tend to use the term
> "workhorse" or "internal workhorse" for this kind of thing.

Indeed, makes sense.

> In the docs, after this patch we have
>=20
> - PQpipelineSync
> - PQsendFlushRequest
> - PQsendPipelineSync
>=20
> Wouldn't it make more sense to add the new function in the middle of the
> two existing ones instead?

Ordering PQsendPipelineSync just after PQpipelineSync is OK by me.
I've applied the patch with all these modifications to move on with
the subject.

> Looking again at the largish comment that's now atop
> pqPipelineSyncInternal(), I think most of it should be removed -- these
> things should be explained in the SGML docs, and I think they are, in
> the "Using Pipeline Mode" section.  We can just have the lines this
> patch is adding.

Hmm.  The first two sentences about being able to submit more commands
to the pipeline are documented in the subsection "Issuing Queries".
The third sentence is implied in the second paragraph of this
subsection.  The 4th paragraph of the comment where sync commands
cannot be issued until all the results from the pipeline have been
consumed is mentioned in the first paragraph in "Using Pipeline Mode".
So you are right that this could be entirely removed.

How about the attached to remove all that, then?
--
Michael

--N5Oy+HUAY7ygRgcX
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="libpq-exec-comments.patch"

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 52d41658c1..152b100624 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3245,23 +3245,6 @@ PQsendPipelineSync(PGconn *conn)
 /*
  * Workhorse function for PQpipelineSync and PQsendPipelineSync.
  *
- * It's legal to start submitting more commands in the pipeline immediately,
- * without waiting for the results of the current pipeline. There's no need to
- * end pipeline mode and start it again.
- *
- * If a command in a pipeline fails, every subsequent command up to and
- * including the result to the Sync message sent by pqPipelineSyncInternal
- * gets set to PGRES_PIPELINE_ABORTED state. If the whole pipeline is
- * processed without error, a PGresult with PGRES_PIPELINE_SYNC is produced.
- *
- * Queries can already have been sent before pqPipelineSyncInternal is called,
- * but pqPipelineSyncInternal needs to be called before retrieving command
- * results.
- *
- * The connection will remain in pipeline mode and unavailable for new
- * synchronous command execution functions until all results from the pipeline
- * are processed by the client.
- *
  * immediate_flush controls if the flush happens immediately after sending the
  * Sync message or not.
  */

--N5Oy+HUAY7ygRgcX--

--rS2+ijLgHRrNbVax
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmWl+MYACgkQnvQgOdby
QH0a4w/9ElgqMHA10ptlmDLiyyUHq2mmcohtTdmvvSVn4VMCTKMpbq5pARtCNW05
8zoKEt0a+qehEziM1gg+CxU9sN5Io4OD3SsAfGaA3ZpqRHcdEvRDmgq9l8vK1WZY
m6F39ee4vK7PEbbP0MVn73oOu7C1w5n0PsB+kwKO0UTyBIfVWuAAA0SjYSOlJ+Wf
OfBaY9jOLIaq/xr+AS7yiZoadAaDB5zEMYDXuaBSMRQaT/qjz7Ar9Ywn3oN68mzw
io+nCOxGPxMxu59VYEgP5uILMpXNGU8YXMCzmVkAWOt4ni7rdS85fZN/jv6tG9me
tyxCvQe5zwWeJzD8VukVDxFSQ92P/MMHTS1we9TGWZ31p1txuST3htXNXvoPfeiQ
A5eXAC8Ua2xcvZqQ3YxkN8TRTmMC8b60LpejWzxWYpa6/BZcyx/VStaMPUKIlSsn
lsb8ZrIpXJwIK9P05x0Zw4VIihNfaIFkWJlaCHM38/swhtlxiyVSUO3f+SGyZKnc
ubBNO1XVAsOImktln8nxJ9wWMYL2RcNOwSXs+ECldUPtD/OdbwpBc6B1keypArhS
+KKbUJZQ/Q3/xIpy1S5AdTAHX3843ieVgHFwSMU/386JXZD6fW9YLGz8UFUbFIZ0
7tUJUizoU8pXddgdMenprGgsttZHhTv8C/HiYSJiFiUIU35Tvbs=
=XUUQ
-----END PGP SIGNATURE-----

--rS2+ijLgHRrNbVax--