postgresql-jdbc.git / summary / log / commit / refs
commit 763ae84d4a78211cee458b1a63dfaeb215fb2efa
Author: Craig Ringer <craig@2ndquadrant.com>
Date: Fri Mar 13 10:15:11 2015 +0000
Disable binary xfer on batches returning generated keys
If a batch that returns generated keys is re-planned mid-execution
because of an input parameter type change, we discard our knowledge
of the field types as obtained at pre-describe time.
This might change a decision about whether to request a result as
binary. If the result was something we'd usually send as as binary
(e.g. int4) then we might request a binary result for the first few
executions. Then if the query gets reparsed and replanned midway
through the batch we'll throw away our knowledge of the fields. Even if
we immediately Describe again, we don't get that result until later, so
we'll still be working with unknown fields when doing parameter binding.
If we don't know the fields we don't specify the result format
requested, so the server sends text.
This means we'll get a mixture of text and binary results for the same
column in the result set, causing errors when we try to decode the
result set row buffer.
There's really no way for us to know in advance whether we can support
binary mode for the result types of every execution, since the result
types can be determined by the input parameters. So just turn it off.
This can happen even if the user doesn't think they're changing the
type. Take the case explained in issue #266, where:
setObject(1, null)
infers type 'text' then a later execution:
setObject(1, "text")
sets type 'varchar' on the driver side. That'll cause a re-plan.
(Arguably we should be retaining our knowledge of the Statement level
describe, not throwing it away on re-plan, but that's a separate patch
to consider).
org/postgresql/core/v3/QueryExecutorImpl.java | 30 +++++++++++++++++-------
org/postgresql/jdbc2/AbstractJdbc2Statement.java | 12 ++++++++--
2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/org/postgresql/core/v3/QueryExecutorImpl.java b/org/postgresql/core/v3/QueryExecutorImpl.java
index 9835e12b..735a227f 100644
--- a/org/postgresql/core/v3/QueryExecutorImpl.java
+++ b/org/postgresql/core/v3/QueryExecutorImpl.java
@@ -1254,7 +1254,9 @@ public class QueryExecutorImpl implements QueryExecutor {
query.unprepare();
processDeadParsedQueries();
- // Remove any cached Field values
+ // Remove any cached Field values. The re-parsed query might report different
+ // fields because input parameter types may result in different type inferences
+ // for unspecified types.
query.setFields(null);
String statementName = null;
@@ -1686,13 +1688,25 @@ public class QueryExecutorImpl implements QueryExecutor {
// A statement describe will also output a RowDescription,
// so don't reissue it here if we've already done so.
//
- if (!noMeta && !describeStatement) {
- // don't send describe if we already have cached the
- // descriptionrow from previous executions
- if (query.getFields() == null) {
- sendDescribePortal(query, portal);
- }
- }
+ if (!noMeta && !describeStatement) {
+ /*
+ * 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.
+ */
+ if (query.getFields() == null) {
+ sendDescribePortal(query, portal);
+ }
+ }
sendExecute(query, portal, rows);
}
diff --git a/org/postgresql/jdbc2/AbstractJdbc2Statement.java b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
index 8df8a676..e801cf51 100644
--- a/org/postgresql/jdbc2/AbstractJdbc2Statement.java
+++ b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
@@ -2873,9 +2873,17 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
if (wantsGeneratedKeysAlways) {
/*
* This batch will return generated keys, tell the executor to
- * expect result rows. We force a Describe later, too.
+ * expect result rows. We also force a Describe later so we know
+ * the size of the results to expect.
+ *
+ * If the parameter type(s) change between batch entries and the
+ * default binary-mode changes we might get mixed binary and text
+ * in a single result set column, which we cannot handle. To prevent
+ * this, disable binary transfer mode in batches that return generated
+ * keys. See GitHub issue #267
*/
- flags = QueryExecutor.QUERY_BOTH_ROWS_AND_STATUS;
+ flags = QueryExecutor.QUERY_BOTH_ROWS_AND_STATUS
+ | QueryExecutor.QUERY_NO_BINARY_TRANSFER;
} else {
// If a batch hasn't specified that it wants generated keys, using the appropriate
// Connection.createStatement(...) interfaces, disallow any result set.
[parent: c1c6edc8301b]