Unnecessary use of v3 Parse/Bind/Execute and named portal sending BEGIN for COPY

[ringerc]

Bug.

PgJDBC uses a v3 protocol Parse/Bind/Execute and may use a named portal ("prepared statement" at the protocol level) when autocommit is off, there is no in-progress transaction and a COPY operation is started via the CopyManager.

This is inefficient but mostly harmless. However, it upsets setups that use PgBouncer in transaction pooling mode with round-robin and/or a server lifetime limit, since the named portal for the BEGIN may vanish out from under PgJDBC. If this happens, the trace log should contain something like:

org.postgresql.core.v3.QueryExecutorImpl receiveErrorResponse
FINEST:  <=BE ErrorMessage(ERROR: prepared statement "S_1" does not exist

This will be the case even if prepareThreshold=0.

There's no good reason to use the extended query protocol for BEGIN so I'll submit a patch to force simple query mode.

[ringerc]

Workarounds:

• Set autocommit on before doing COPY; or
• Send an explicit BEGIN statement on the connection before beginning the COPY

[ringerc]

It looks like there's an opportunity to detect if the connection has been put in read-only state and send the read-only begin instead. I haven't made that change at the same time.

[ringerc]

Will send a PR, but here's the patch I expect will do the trick

diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
index c944ca03..20ce1762 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
@@ -630,8 +630,12 @@ public class QueryExecutorImpl extends QueryExecutorBase {
       };
 
       try {
-        sendOneQuery(beginTransactionQuery, SimpleQuery.NO_PARAMETERS, 0, 0,
-            QueryExecutor.QUERY_NO_METADATA);
+        /* Send BEGIN with simple protocol preferred */
+        int beginFlags = QueryExecutor.QUERY_NO_METADATA
+                         | QueryExecutor.QUERY_ONESHOT
+                         | QueryExecutor.QUERY_EXECUTE_AS_SIMPLE;
+        beginFlags = updateQueryMode(beginFlags);
+        sendOneQuery(beginTransactionQuery, SimpleQuery.NO_PARAMETERS, 0, 0, beginFlags);
         sendSync();
         processResults(handler, 0);
         estimatedReceiveBufferBytes = 0;

[vlsi]

There's no good reason to use the extended query protocol for BEGIN

Is it baked with the measurements?

[ringerc]

Created as #1639

Backed by measurements? No, but we don't use extended query protocol for BEGIN anywhere else unless forced, and this was clearly an oversight. And 3 protocol messages is more work than 1. I know colleagues have done work that's found that the extended query protocol adds non-trivial overhead for simple statements, but I cannot presently point to specific benchmarks etc.

Look at sendQueryPreamble. This patch makes the COPY setup path more consistent with the BEGIN we send there.

[ringerc]

Note that this is very much a bug fix - we use extended query protocol even if the query mode is set to simple. We may use named portals (prepared statements) even if prepareThreshold=0. We're not respecting the configuration here.

It just so happens that it's a pretty harmless bug unless you're using a weird pooler configuration.

[ringerc]

Thanks @davecramer :) much appreciated. Tried to supply a decent report to make it easy.