From e08911369e5d3ba904ff9fdc42535ba47a6dc837 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 20 Nov 2013 19:45:24 +0000 Subject: [PATCH] Only use RETURN_GENERATED_KEYS on inserts. This works around "unsophisticated" Postgres JDBC drivers which blindly tack "RETURNING *" onto a query if RETURN_GENERATED_KEYS is supplied. --- .../com/samskivert/depot/DepotRepository.java | 4 +- .../com/samskivert/depot/impl/SQLBuilder.java | 130 ++++++++++-------- 2 files changed, 76 insertions(+), 58 deletions(-) diff --git a/src/main/java/com/samskivert/depot/DepotRepository.java b/src/main/java/com/samskivert/depot/DepotRepository.java index c78bcbc..de9ddf3 100644 --- a/src/main/java/com/samskivert/depot/DepotRepository.java +++ b/src/main/java/com/samskivert/depot/DepotRepository.java @@ -321,7 +321,7 @@ public abstract class DepotRepository } builder.newQuery(new InsertClause(pClass, _result, identityFields)); - PreparedStatement stmt = builder.prepare(conn); + PreparedStatement stmt = builder.prepareInsert(conn); int mods = stmt.executeUpdate(); // run any post-factum value generators and potentially generate our key if (_key == null) { @@ -579,7 +579,7 @@ public abstract class DepotRepository } builder.newQuery(new InsertClause(pClass, _result, identityFields)); - PreparedStatement stmt = builder.prepare(conn); + PreparedStatement stmt = builder.prepareInsert(conn); int mods = stmt.executeUpdate(); // run any post-factum value generators and potentially generate our key diff --git a/src/main/java/com/samskivert/depot/impl/SQLBuilder.java b/src/main/java/com/samskivert/depot/impl/SQLBuilder.java index 303c339..1f8b39e 100644 --- a/src/main/java/com/samskivert/depot/impl/SQLBuilder.java +++ b/src/main/java/com/samskivert/depot/impl/SQLBuilder.java @@ -62,30 +62,19 @@ public abstract class SQLBuilder public PreparedStatement prepare (Connection conn) throws SQLException { - checkState(_buildVisitor != null, "Cannot prepare query until it's been built."); - - PreparedStatement stmt = conn.prepareStatement( - _buildVisitor.getQuery(), PreparedStatement.RETURN_GENERATED_KEYS); - - int argIx = 1; - for (BuildVisitor.Bindable bindable : _buildVisitor.getBindables()) { - try { - bindable.doBind(conn, stmt, argIx); - } catch (Exception e) { - log.warning("Failed to bind statement argument", "argIx", argIx, e); - } - argIx ++; - } - - if (PersistenceContext.DEBUG) { - log.info("SQL: " + stmt.toString()); - } - - return stmt; + return prepare(conn, conn.prepareStatement(buildQuery())); } - protected String nullify (String str) { - return (str != null && str.length() > 0) ? str : null; + /** + * A variant of {@link #prepare} that must be used for INSERTs. Due to a Postgres JDBC driver + * bug, we need to restrict the use of RETURN_GENERATED_KEYS to insert queries only, which + * means we need to know when we're doing an insert. + */ + public PreparedStatement prepareInsert (Connection conn) + throws SQLException + { + return prepare( + conn, conn.prepareStatement(buildQuery(), PreparedStatement.RETURN_GENERATED_KEYS)); } /** @@ -129,6 +118,69 @@ public abstract class SQLBuilder return coldef; } + /** + * Add full-text search capabilities, as defined by the provided {@link FullTextIndex}, on + * the table associated with the given {@link DepotMarshaller}. This is a highly database + * specific operation and must thus be implemented by each dialect subclass. + * + * @see FullTextIndex + */ + public abstract boolean addFullTextSearch ( + Connection conn, DepotMarshaller marshaller, FullTextIndex fts) + throws SQLException; + + /** + * Return true if the supplied column is an internal consideration of this {@link SQLBuilder}, + * e.g. PostgreSQL's full text search data is stored in a table column that should otherwise + * not be visible to Depot; this method helps mask it. + */ + public abstract boolean isPrivateColumn ( + String column, Map fullTextIndexes); + + /** + * Return true if the supplied index is an internal consideration of this {@link SQLBuilder}, + * e.g. PostgreSQL automatically creates indexes that end in _key for unique columns, and its + * primary keys have indices ending in _pkey. + */ + public abstract boolean isPrivateIndex ( + String index, Map fullTextIndexes); + + /** + * Figure out what full text search indexes already exist on this table and add the names of + * those indexes to the supplied target set. + */ + public abstract void getFtsIndexes ( + Iterable columns, Iterable indexes, Set target); + + protected String buildQuery () { + checkState(_buildVisitor != null, "Cannot prepare query until it's been built."); + return _buildVisitor.getQuery(); + } + + protected PreparedStatement prepare (Connection conn, PreparedStatement stmt) + throws SQLException + { + int argIx = 1; + for (BuildVisitor.Bindable bindable : _buildVisitor.getBindables()) { + try { + bindable.doBind(conn, stmt, argIx); + } catch (Exception e) { + log.warning("Failed to bind statement argument", "argIx", argIx, e); + } + argIx ++; + } + + if (PersistenceContext.DEBUG) { + log.info("SQL: " + stmt.toString()); + } + + return stmt; + } + + protected String nullify (String str) { + return (str != null && str.length() > 0) ? str : null; + } + protected void maybeMutateForPrimitive (Field field, ColumnDefinition coldef) { // Java primitive types cannot be null, so we provide a default value for these columns @@ -183,40 +235,6 @@ public abstract class SQLBuilder return "false"; } - /** - * Add full-text search capabilities, as defined by the provided {@link FullTextIndex}, on - * the table associated with the given {@link DepotMarshaller}. This is a highly database - * specific operation and must thus be implemented by each dialect subclass. - * - * @see FullTextIndex - */ - public abstract boolean addFullTextSearch ( - Connection conn, DepotMarshaller marshaller, FullTextIndex fts) - throws SQLException; - - /** - * Return true if the supplied column is an internal consideration of this {@link SQLBuilder}, - * e.g. PostgreSQL's full text search data is stored in a table column that should otherwise - * not be visible to Depot; this method helps mask it. - */ - public abstract boolean isPrivateColumn ( - String column, Map fullTextIndexes); - - /** - * Return true if the supplied index is an internal consideration of this {@link SQLBuilder}, - * e.g. PostgreSQL automatically creates indexes that end in _key for unique columns, and its - * primary keys have indices ending in _pkey. - */ - public abstract boolean isPrivateIndex ( - String index, Map fullTextIndexes); - - /** - * Figure out what full text search indexes already exist on this table and add the names of - * those indexes to the supplied target set. - */ - public abstract void getFtsIndexes ( - Iterable columns, Iterable indexes, Set target); - /** * Overridden by subclasses to create a dialect-specific {@link BuildVisitor}. */