postgresql-jdbc.git / summary / log / commit / refs

commit    a6bd36faaedc779f932fa76f52bab9550f0fcd6d
Author:   Craig Ringer <craig@2ndquadrant.com>
Date:     Mon Mar 16 07:14:12 2015 +0000

    Always Describe a query in a batch that returns generated keys
    
    This is a workaround for issue #267, where we unprepare a statement
    in a batch when the parameter types change but we expect to still
    know the field types in the statement we just unprepared.
    
    It's much more complicated than that, see
    https://github.com/pgjdbc/pgjdbc/issues/267 for the gory details.


 org/postgresql/core/QueryExecutor.java           |  7 ++++++
 org/postgresql/core/v3/QueryExecutorImpl.java    | 29 +++++++++++++++---------
 org/postgresql/jdbc2/AbstractJdbc2Statement.java |  6 +++++
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/org/postgresql/core/QueryExecutor.java b/org/postgresql/core/QueryExecutor.java
index 10bd5025..4f860b1c 100644
--- a/org/postgresql/core/QueryExecutor.java
+++ b/org/postgresql/core/QueryExecutor.java
@@ -88,6 +88,13 @@ public interface QueryExecutor {
      * both the ResultSet and associated update count from the command status.
      */
     static int QUERY_BOTH_ROWS_AND_STATUS = 64;
+    
+    /**
+     * Force this query to be described at each execution. This is done in
+     * pipelined batches where we might need to detect mismatched result
+     * types.
+     */
+    static int QUERY_FORCE_DESCRIBE_PORTAL = 128;
 
     /**
      * Flag to disable batch execution when we expect results (generated keys)
diff --git a/org/postgresql/core/v3/QueryExecutorImpl.java b/org/postgresql/core/v3/QueryExecutorImpl.java
index 735a227f..76d5e34d 100644
--- a/org/postgresql/core/v3/QueryExecutorImpl.java
+++ b/org/postgresql/core/v3/QueryExecutorImpl.java
@@ -1624,6 +1624,7 @@ public class QueryExecutorImpl implements QueryExecutor {
         boolean usePortal = (flags & QueryExecutor.QUERY_FORWARD_CURSOR) != 0 && !noResults && !noMeta && fetchSize > 0 && !describeOnly;
         boolean oneShot = (flags & QueryExecutor.QUERY_ONESHOT) != 0 && !usePortal;
         boolean noBinaryTransfer = (flags & QUERY_NO_BINARY_TRANSFER) != 0;
+        boolean forceDescribePortal = (flags & QUERY_FORCE_DESCRIBE_PORTAL) != 0;
 
         // Work out how many rows to fetch in this pass.
 
@@ -1692,18 +1693,24 @@ public class QueryExecutorImpl implements QueryExecutor {
 			/*
 			 * don't send describe if we already have cached the row description
 			 * from previous executions
-			 *
-			 * XXX Clearing the fields / unpreparing the query is incorrect, see
-			 * bug #267. We might clear the cached fields in a later execution
-			 * of this query if the bind parameter types change, but we're
-			 * assuming here that they'll still be valid when we come to process
-			 * the results of this query, so we don't send a new describe here.
-			 * We re-describe after the fields are cleared, but the result of
-			 * that gets processed after processing the results from earlier
-			 * executions that we didn't describe because we didn't think we had
-			 * to.
+			 * 
+			 * XXX Clearing the fields / unpreparing the query (in sendParse) is
+			 * incorrect, see bug #267. We might clear the cached fields in a
+			 * later execution of this query if the bind parameter types change,
+			 * but we're assuming here that they'll still be valid when we come
+			 * to process the results of this query, so we don't send a new
+			 * describe here. We re-describe after the fields are cleared, but
+			 * the result of that gets processed after processing the results
+			 * from earlier executions that we didn't describe because we didn't
+			 * think we had to.
+			 * 
+			 * To work around this, force a Describe at each execution in
+			 * batches where this can be a problem. It won't cause more round
+			 * trips so the performance impact is low, and it'll ensure that the
+			 * field information available when we decoded the results. This
+			 * is undeniably a hack, but there aren't many good alternatives.
 			 */
-			if (query.getFields() == null) {
+			if (query.getFields() == null || forceDescribePortal) {
 				sendDescribePortal(query, portal);
 			}
 		}
diff --git a/org/postgresql/jdbc2/AbstractJdbc2Statement.java b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
index e801cf51..adbd65e1 100644
--- a/org/postgresql/jdbc2/AbstractJdbc2Statement.java
+++ b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
@@ -2905,6 +2905,12 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
             // we'll be able to queue up before we risk a deadlock.
             // (see v3.QueryExecutorImpl's MAX_BUFFERED_RECV_BYTES)
             preDescribe = wantsGeneratedKeysAlways && !queries[0].isStatementDescribed();
+            /*
+             * It's also necessary to force a Describe on the first execution of the
+             * new statement, even though we already described it, to work around
+             * bug #267.
+             */
+            flags |= QueryExecutor.QUERY_FORCE_DESCRIBE_PORTAL;
         }
 
         if (connection.getAutoCommit())


[parent: 4d2b0466e677]