diff --git a/src/java/com/samskivert/depot/DepotRepository.java b/src/java/com/samskivert/depot/DepotRepository.java index e110c30..0580191 100644 --- a/src/java/com/samskivert/depot/DepotRepository.java +++ b/src/java/com/samskivert/depot/DepotRepository.java @@ -21,7 +21,6 @@ package com.samskivert.depot; import java.sql.Connection; -import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Timestamp; @@ -39,8 +38,6 @@ import com.samskivert.util.ArrayUtil; import com.samskivert.jdbc.ConnectionProvider; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; - import com.samskivert.depot.clause.FieldOverride; import com.samskivert.depot.clause.InsertClause; import com.samskivert.depot.clause.QueryClause; @@ -459,19 +456,13 @@ public abstract class DepotRepository builder.newQuery(new InsertClause(pClass, _result, identityFields)); - PreparedStatement stmt = builder.prepare(conn); - try { - int mods = stmt.executeUpdate(); - // run any post-factum value generators and potentially generate our key - if (_key == null) { - marsh.generateFieldValues(conn, liaison, _result, true); - updateKey(marsh.getPrimaryKey(_result, false)); - } - return mods; - - } finally { - JDBCUtil.close(stmt); + int mods = builder.prepare(conn).executeUpdate(); + // run any post-factum value generators and potentially generate our key + if (_key == null) { + marsh.generateFieldValues(conn, liaison, _result, true); + updateKey(marsh.getPrimaryKey(_result, false)); } + return mods; } }); } @@ -698,44 +689,36 @@ public abstract class DepotRepository _ctx.invoke(new CachingModifier(record, key, key) { @Override protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement stmt = null; - try { - if (_key != null) { - // run the update - stmt = builder.prepare(conn); - int mods = stmt.executeUpdate(); - if (mods > 0) { - // if it succeeded, we're done - return mods; - } - JDBCUtil.close(stmt); + if (_key != null) { + // run the update + int mods = builder.prepare(conn).executeUpdate(); + if (mods > 0) { + // if it succeeded, we're done + return mods; } - - // if the update modified zero rows or the primary key was unset, insert - Set identityFields = Collections.emptySet(); - if (_key == null) { - // first, set any auto-generated column values - identityFields = marsh.generateFieldValues(conn, liaison, _result, false); - // update our modifier's key so that it can cache our results - updateKey(marsh.getPrimaryKey(_result, false)); - } - - builder.newQuery(new InsertClause(pClass, _result, identityFields)); - - stmt = builder.prepare(conn); - int mods = stmt.executeUpdate(); - - // run any post-factum value generators and potentially generate our key - if (_key == null) { - marsh.generateFieldValues(conn, liaison, _result, true); - updateKey(marsh.getPrimaryKey(_result, false)); - } - created[0] = true; - return mods; - - } finally { - JDBCUtil.close(stmt); } + + // if the update modified zero rows or the primary key was unset, insert + Set identityFields = Collections.emptySet(); + if (_key == null) { + + // first, set any auto-generated column values + identityFields = marsh.generateFieldValues(conn, liaison, _result, false); + // update our modifier's key so that it can cache our results + updateKey(marsh.getPrimaryKey(_result, false)); + } + + builder.newQuery(new InsertClause(pClass, _result, identityFields)); + + int mods = builder.prepare(conn).executeUpdate(); + + // run any post-factum value generators and potentially generate our key + if (_key == null) { + marsh.generateFieldValues(conn, liaison, _result, true); + updateKey(marsh.getPrimaryKey(_result, false)); + } + created[0] = true; + return mods; } }); return created[0]; @@ -821,12 +804,7 @@ public abstract class DepotRepository return _ctx.invoke(new Modifier(invalidator) { @Override protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement stmt = builder.prepare(conn); - try { - return stmt.executeUpdate(); - } finally { - JDBCUtil.close(stmt); - } + return builder.prepare(conn).executeUpdate(); } }); } @@ -855,12 +833,7 @@ public abstract class DepotRepository return _ctx.invoke(new Modifier(invalidator) { @Override protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement stmt = builder.prepare(conn); - try { - return stmt.executeUpdate(); - } finally { - JDBCUtil.close(stmt); - } + return builder.prepare(conn).executeUpdate(); } }); } diff --git a/src/java/com/samskivert/depot/PersistenceContext.java b/src/java/com/samskivert/depot/PersistenceContext.java index a112bae..1e6812f 100644 --- a/src/java/com/samskivert/depot/PersistenceContext.java +++ b/src/java/com/samskivert/depot/PersistenceContext.java @@ -27,6 +27,7 @@ import java.util.Set; import java.io.Serializable; import java.sql.Connection; import java.sql.SQLException; +import java.sql.Statement; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -37,6 +38,7 @@ import com.samskivert.util.StringUtil; import com.samskivert.jdbc.ConnectionProvider; import com.samskivert.jdbc.DatabaseLiaison; +import com.samskivert.jdbc.JDBCUtil; import com.samskivert.jdbc.LiaisonRegistry; import com.samskivert.depot.CacheAdapter.CacheCategory; @@ -539,6 +541,10 @@ public class PersistenceContext throw new DatabaseException(pe.getMessage(), pe.getCause()); } + // wrap the connection in a proxy that will collect all opened statements + List stmts = Lists.newArrayListWithCapacity(1); + conn = JDBCUtil.makeCollector(conn, stmts); + // TEMP: we synchronize on the connection to cooperate with SimpleRepository when used in // conjunction with a StaticConnectionProvider; at some point we'll switch to standard JDBC // connection pooling which will block in getConnection() instead of returning a connection @@ -547,7 +553,18 @@ public class PersistenceContext long preInvoke = System.nanoTime(); try { // invoke our database operation - T value = op.invoke(this, conn, _liaison); + T value; + try { + value = op.invoke(this, conn, _liaison); + } finally { + // close all opened statements; if any close fails, abort the close process as + // the whole connection is now unusable and will be discarded + for (Statement stmt : stmts) { + if (!stmt.isClosed()) { + stmt.close(); + } + } + } // if auto-commit is off and this is a write operation, push it through if (!isReadOnly && !conn.getAutoCommit()) { conn.commit(); diff --git a/src/java/com/samskivert/depot/impl/DepotMarshaller.java b/src/java/com/samskivert/depot/impl/DepotMarshaller.java index 47b821b..ccf8863 100644 --- a/src/java/com/samskivert/depot/impl/DepotMarshaller.java +++ b/src/java/com/samskivert/depot/impl/DepotMarshaller.java @@ -44,8 +44,6 @@ import com.samskivert.util.Tuple; import com.samskivert.jdbc.ColumnDefinition; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; - import com.samskivert.depot.DatabaseException; import com.samskivert.depot.Key; import com.samskivert.depot.PersistenceContext; @@ -711,12 +709,7 @@ public class DepotMarshaller throws SQLException { if (builder.newQuery(clause)) { - PreparedStatement stmt = builder.prepare(conn); - try { - return stmt.executeUpdate(); - } finally { - JDBCUtil.close(stmt); - } + return builder.prepare(conn).executeUpdate(); } return 0; } diff --git a/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java b/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java index 65dd5a8..cca5aba 100644 --- a/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java +++ b/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java @@ -28,8 +28,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; - import com.samskivert.depot.DatabaseException; import com.samskivert.depot.Key; import com.samskivert.depot.PersistenceContext; @@ -76,19 +74,15 @@ public class FindAllKeysQuery extends Query> keys = new XArrayList>(); PreparedStatement stmt = _builder.prepare(conn); - try { - ResultSet rs = stmt.executeQuery(); - while (rs.next()) { - keys.add(_marsh.makePrimaryKey(rs)); - } - // TODO: cache this result? - if (PersistenceContext.CACHE_DEBUG) { - log.info("Loaded " + _marsh.getTableName(), "count", keys.size()); - } - return keys; - } finally { - JDBCUtil.close(stmt); + ResultSet rs = stmt.executeQuery(); + while (rs.next()) { + keys.add(_marsh.makePrimaryKey(rs)); } + // TODO: cache this result? + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + _marsh.getTableName(), "count", keys.size()); + } + return keys; } // from Query diff --git a/src/java/com/samskivert/depot/impl/FindAllQuery.java b/src/java/com/samskivert/depot/impl/FindAllQuery.java index 5cee113..162abde 100644 --- a/src/java/com/samskivert/depot/impl/FindAllQuery.java +++ b/src/java/com/samskivert/depot/impl/FindAllQuery.java @@ -36,8 +36,6 @@ import com.google.common.collect.Sets; import com.google.common.collect.Maps; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; - import com.samskivert.depot.CacheAdapter.CacheCategory; import com.samskivert.depot.DatabaseException; import com.samskivert.depot.DepotRepository.CacheStrategy; @@ -132,13 +130,9 @@ public abstract class FindAllQuery extends Query extends Query result = new XArrayList(); SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select)); builder.newQuery(_select); - PreparedStatement stmt = builder.prepare(conn); - try { - ResultSet rs = stmt.executeQuery(); - while (rs.next()) { - result.add(_marsh.createObject(rs)); - } - } finally { - JDBCUtil.close(stmt); + ResultSet rs = builder.prepare(conn).executeQuery(); + while (rs.next()) { + result.add(_marsh.createObject(rs)); } _explicitQueries++; if (PersistenceContext.CACHE_DEBUG) { @@ -348,39 +337,33 @@ public abstract class FindAllQuery extends Query> got = Sets.newHashSet(); - ResultSet rs = stmt.executeQuery(); - int cnt = 0, dups = 0; - while (rs.next()) { - T obj = _marsh.createObject(rs); - Key key = _marsh.getPrimaryKey(obj); - if (entities.put(key, obj) != null) { - dups++; - } - ctx.cacheStore(CacheCategory.RECORD, new KeyCacheKey(key), obj.clone()); - got.add(key); - cnt++; + Set> got = Sets.newHashSet(); + ResultSet rs = builder.prepare(conn).executeQuery(); + int cnt = 0, dups = 0; + while (rs.next()) { + T obj = _marsh.createObject(rs); + Key key = _marsh.getPrimaryKey(obj); + if (entities.put(key, obj) != null) { + dups++; } - // if we get more results than we planned, or if we're doing a two-phase query and got - // fewer, then complain - if (cnt > keys.size() || (origStmt != null && cnt < keys.size())) { - log.warning("Row count mismatch in second pass", "origQuery", origStmt, - // we need toString() here or StringUtil will get smart and dump our - // KeySet using its iterator which results in verbosity - "wanted", KeySet.newKeySet(_type, keys).toString(), - "got", KeySet.newKeySet(_type, got).toString(), - "dups", dups, new Exception()); - } - - if (PersistenceContext.CACHE_DEBUG) { - log.info("Cached " + _marsh.getTableName(), "count", cnt); - } - - } finally { - JDBCUtil.close(stmt); + ctx.cacheStore(CacheCategory.RECORD, new KeyCacheKey(key), obj.clone()); + got.add(key); + cnt++; } + // if we get more results than we planned, or if we're doing a two-phase query and got + // fewer, then complain + if (cnt > keys.size() || (origStmt != null && cnt < keys.size())) { + log.warning("Row count mismatch in second pass", "origQuery", origStmt, + // we need toString() here or StringUtil will get smart and dump our + // KeySet using its iterator which results in verbosity + "wanted", KeySet.newKeySet(_type, keys).toString(), "got", KeySet.newKeySet( + _type, got).toString(), "dups", dups, new Exception()); + } + + if (PersistenceContext.CACHE_DEBUG) { + log.info("Cached " + _marsh.getTableName(), "count", cnt); + } + } protected String keysToString (Iterable> keySet) diff --git a/src/java/com/samskivert/depot/impl/FindOneQuery.java b/src/java/com/samskivert/depot/impl/FindOneQuery.java index 7fee5e0..f6210f4 100644 --- a/src/java/com/samskivert/depot/impl/FindOneQuery.java +++ b/src/java/com/samskivert/depot/impl/FindOneQuery.java @@ -21,13 +21,10 @@ package com.samskivert.depot.impl; import java.sql.Connection; -import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; - import com.samskivert.depot.CacheKey; import com.samskivert.depot.DatabaseException; import com.samskivert.depot.DepotRepository; @@ -82,40 +79,34 @@ public class FindOneQuery extends Query public T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement stmt = _builder.prepare(conn); - try { - // load up the record in question - T result = null; - ResultSet rs = stmt.executeQuery(); - if (rs.next()) { - result = _marsh.createObject(rs); - } - // TODO: if (rs.next()) issue warning? - rs.close(); - - // potentially cache the result - CacheKey key = getCacheKey(); - if (key == null) { - // no row-specific cache key was given, if we can, create a key from the record - if (result != null && _marsh.hasPrimaryKey()) { - key = new KeyCacheKey(_marsh.getPrimaryKey(result)); - } - } - if (PersistenceContext.CACHE_DEBUG) { - log.info("Loaded " + (key != null ? key : _marsh.getTableName())); - } - if (key != null) { - ctx.cacheStore(CacheCategory.RECORD, key, (result != null) ? result.clone() : null); - if (PersistenceContext.CACHE_DEBUG) { - log.info("Cached " + key); - } - } - - return result; - - } finally { - JDBCUtil.close(stmt); + // load up the record in question + T result = null; + ResultSet rs = _builder.prepare(conn).executeQuery(); + if (rs.next()) { + result = _marsh.createObject(rs); } + // TODO: if (rs.next()) issue warning? + rs.close(); + + // potentially cache the result + CacheKey key = getCacheKey(); + if (key == null) { + // no row-specific cache key was given, if we can, create a key from the record + if (result != null && _marsh.hasPrimaryKey()) { + key = new KeyCacheKey(_marsh.getPrimaryKey(result)); + } + } + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + (key != null ? key : _marsh.getTableName())); + } + if (key != null) { + ctx.cacheStore(CacheCategory.RECORD, key, (result != null) ? result.clone() : null); + if (PersistenceContext.CACHE_DEBUG) { + log.info("Cached " + key); + } + } + + return result; } // from Operation diff --git a/src/java/com/samskivert/depot/impl/MySQLBuilder.java b/src/java/com/samskivert/depot/impl/MySQLBuilder.java index 0b855ab..35b26e6 100644 --- a/src/java/com/samskivert/depot/impl/MySQLBuilder.java +++ b/src/java/com/samskivert/depot/impl/MySQLBuilder.java @@ -25,12 +25,10 @@ import java.sql.Clob; import java.sql.Connection; import java.sql.Date; import java.sql.SQLException; -import java.sql.Statement; import java.sql.Time; import java.sql.Timestamp; import java.util.Set; -import com.samskivert.jdbc.JDBCUtil; import com.samskivert.util.Tuple; import com.samskivert.depot.PersistentRecord; @@ -206,13 +204,8 @@ public class MySQLBuilder } update.append(")"); - Statement stmt = conn.createStatement(); - try { - log.info("Adding full-text search index: ftsIx_" + fts.name()); - stmt.executeUpdate(update.toString()); - } finally { - JDBCUtil.close(stmt); - } + log.info("Adding full-text search index: ftsIx_" + fts.name()); + conn.createStatement().executeUpdate(update.toString()); return true; } diff --git a/src/java/com/samskivert/depot/impl/Operation.java b/src/java/com/samskivert/depot/impl/Operation.java index 6d6bf31..687678a 100644 --- a/src/java/com/samskivert/depot/impl/Operation.java +++ b/src/java/com/samskivert/depot/impl/Operation.java @@ -22,6 +22,7 @@ package com.samskivert.depot.impl; import java.sql.Connection; import java.sql.SQLException; +import java.sql.Statement; import com.samskivert.depot.PersistenceContext; import com.samskivert.depot.Stats; @@ -38,7 +39,9 @@ public interface Operation public boolean isReadOnly (); /** - * Performs the actual JDBC interactions associated with this operation. + * Performs the actual JDBC interactions associated with this operation. Any {@link Statement} + * instances created with the given connection will be closed automatically after this method + * returns. The operation need not close them itself. */ public T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException; diff --git a/src/java/com/samskivert/depot/impl/PostgreSQLBuilder.java b/src/java/com/samskivert/depot/impl/PostgreSQLBuilder.java index 2574d86..e5f52b9 100644 --- a/src/java/com/samskivert/depot/impl/PostgreSQLBuilder.java +++ b/src/java/com/samskivert/depot/impl/PostgreSQLBuilder.java @@ -31,7 +31,6 @@ import java.sql.Timestamp; import java.util.Set; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; import com.samskivert.jdbc.LiaisonRegistry; import com.samskivert.util.ArrayUtil; import com.samskivert.util.StringUtil; @@ -185,18 +184,12 @@ public class PostgreSQLBuilder append(liaison.columnSQL(column)).append(")"); Statement stmt = conn.createStatement(); - try { - log.info( - "Adding full-text search column, index and trigger: " + column + ", " + - index + ", " + trigger); - liaison.addColumn(conn, table, column, "TSVECTOR", true); - stmt.executeUpdate(initColumn.toString()); - stmt.executeUpdate(createIndex.toString()); - stmt.executeUpdate(createTrigger.toString()); - - } finally { - JDBCUtil.close(stmt); - } + log.info("Adding full-text search column, index and trigger: " + column + ", " + + index + ", " + trigger); + liaison.addColumn(conn, table, column, "TSVECTOR", true); + stmt.executeUpdate(initColumn.toString()); + stmt.executeUpdate(createIndex.toString()); + stmt.executeUpdate(createTrigger.toString()); return true; } diff --git a/src/java/com/samskivert/depot/impl/TableValueGenerator.java b/src/java/com/samskivert/depot/impl/TableValueGenerator.java index 18dee12..ea8135d 100644 --- a/src/java/com/samskivert/depot/impl/TableValueGenerator.java +++ b/src/java/com/samskivert/depot/impl/TableValueGenerator.java @@ -30,7 +30,6 @@ import com.samskivert.depot.annotation.TableGenerator; import com.samskivert.jdbc.ColumnDefinition; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; /** * Generates primary keys using an external table . @@ -69,106 +68,83 @@ public class TableValueGenerator extends ValueGenerator new String[] { _pkColumnName }); // and also that there's a row in it for us - PreparedStatement stmt = null; - try { - stmt = conn.prepareStatement( - " SELECT * FROM " + liaison.tableSQL(_valueTable) + - " WHERE " + liaison.columnSQL(_pkColumnName) + " = ?"); - stmt.setString(1, _pkColumnValue); - if (stmt.executeQuery().next()) { - return; - } - - JDBCUtil.close(stmt); - stmt = null; - - int initialValue = _initialValue; - if (_migrateIfExists) { - Integer max = getFieldMaximum(conn, liaison); - if (max != null) { - initialValue = 1 + max.intValue(); - } - } - - stmt = conn.prepareStatement( - " INSERT INTO " + liaison.tableSQL(_valueTable) + " (" + - liaison.columnSQL(_pkColumnName) + ", " + liaison.columnSQL(_valueColumnName) + - ") VALUES (?, ?)"); - stmt.setString(1, _pkColumnValue); - stmt.setInt(2, initialValue); - stmt.executeUpdate(); - - } finally { - JDBCUtil.close(stmt); + PreparedStatement stmt = conn.prepareStatement( + " SELECT * FROM " + liaison.tableSQL(_valueTable) + + " WHERE " + liaison.columnSQL(_pkColumnName) + " = ?"); + stmt.setString(1, _pkColumnValue); + if (stmt.executeQuery().next()) { + return; } + + int initialValue = _initialValue; + if (_migrateIfExists) { + Integer max = getFieldMaximum(conn, liaison); + if (max != null) { + initialValue = 1 + max.intValue(); + } + } + + stmt = conn.prepareStatement( + " INSERT INTO " + liaison.tableSQL(_valueTable) + " (" + + liaison.columnSQL(_pkColumnName) + ", " + liaison.columnSQL(_valueColumnName) + + ") VALUES (?, ?)"); + stmt.setString(1, _pkColumnValue); + stmt.setInt(2, initialValue); + stmt.executeUpdate(); } @Override // from ValueGenerator public void delete (Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement stmt = null; - try { - stmt = conn.prepareStatement( - " DELETE FROM " + liaison.tableSQL(_valueTable) + - " WHERE " + liaison.columnSQL(_pkColumnName) + " = ?"); - stmt.setString(1, _pkColumnValue); - stmt.executeUpdate(); - - } finally { - JDBCUtil.close(stmt); - } + PreparedStatement stmt = conn.prepareStatement( + " DELETE FROM " + liaison.tableSQL(_valueTable) + + " WHERE " + liaison.columnSQL(_pkColumnName) + " = ?"); + stmt.setString(1, _pkColumnValue); + stmt.executeUpdate(); } @Override // from ValueGenerator public int nextGeneratedValue (Connection conn, DatabaseLiaison liaison) throws SQLException { - PreparedStatement readStatement = null; - PreparedStatement writeStatement = null; - try { - // TODO: Make this lockless! - readStatement = conn.prepareStatement( - " SELECT " + liaison.columnSQL(_valueColumnName) + - " FROM " + liaison.tableSQL(_valueTable) + - " WHERE " + liaison.columnSQL(_pkColumnName) + " = ? "); - readStatement.setString(1, _pkColumnValue); + // TODO: Make this lockless! + PreparedStatement readStatement = conn.prepareStatement( + " SELECT " + liaison.columnSQL(_valueColumnName) + + " FROM " + liaison.tableSQL(_valueTable) + + " WHERE " + liaison.columnSQL(_pkColumnName) + " = ? "); + readStatement.setString(1, _pkColumnValue); - writeStatement = conn.prepareStatement( - " UPDATE " + liaison.tableSQL(_valueTable) + - " SET " + liaison.columnSQL(_valueColumnName) + " = ? " + - " WHERE " + liaison.columnSQL(_pkColumnName) + " = ? " + - " AND " + liaison.columnSQL(_valueColumnName) + " = ? "); + PreparedStatement writeStatement = conn.prepareStatement( + " UPDATE " + liaison.tableSQL(_valueTable) + + " SET " + liaison.columnSQL(_valueColumnName) + " = ? " + + " WHERE " + liaison.columnSQL(_pkColumnName) + " = ? " + + " AND " + liaison.columnSQL(_valueColumnName) + " = ? "); - for (int tries = 0; tries < 10; tries ++) { - // execute the query - ResultSet rs = readStatement.executeQuery(); - if (!rs.next()) { - throw new SQLException( - "Failed to find next primary key value [table=" + _valueTable + - ", column=" + _valueColumnName + "]"); - } - // fetch the next available value - int val = rs.getInt(1); - - // claim this value locklessly - writeStatement.setInt(1, val + _allocationSize); - writeStatement.setString(2, _pkColumnValue); - writeStatement.setInt(3, val); - - // if we modified a row, we know we and nobody else got this particular value! - if (writeStatement.executeUpdate() == 1) { - return val; - } - // else try again + for (int tries = 0; tries < 10; tries ++) { + // execute the query + ResultSet rs = readStatement.executeQuery(); + if (!rs.next()) { + throw new SQLException( + "Failed to find next primary key value [table=" + _valueTable + + ", column=" + _valueColumnName + "]"); } - throw new SQLException( - "Failed to claim next primary key value in 10 attempts [table=" + _valueTable + - ", column=" + _valueColumnName + "]"); + // fetch the next available value + int val = rs.getInt(1); - } finally { - JDBCUtil.close(readStatement); - JDBCUtil.close(writeStatement); + // claim this value locklessly + writeStatement.setInt(1, val + _allocationSize); + writeStatement.setString(2, _pkColumnValue); + writeStatement.setInt(3, val); + + // if we modified a row, we know we and nobody else got this particular value! + if (writeStatement.executeUpdate() == 1) { + return val; + } + // else try again } + throw new SQLException( + "Failed to claim next primary key value in 10 attempts [table=" + _valueTable + + ", column=" + _valueColumnName + "]"); } /** diff --git a/src/java/com/samskivert/depot/impl/ValueGenerator.java b/src/java/com/samskivert/depot/impl/ValueGenerator.java index c02c1bc..2e176f4 100644 --- a/src/java/com/samskivert/depot/impl/ValueGenerator.java +++ b/src/java/com/samskivert/depot/impl/ValueGenerator.java @@ -26,7 +26,6 @@ import java.sql.SQLException; import java.sql.Statement; import com.samskivert.jdbc.DatabaseLiaison; -import com.samskivert.jdbc.JDBCUtil; import com.samskivert.depot.annotation.GeneratedValue; import static com.samskivert.depot.Log.log; @@ -85,24 +84,19 @@ public abstract class ValueGenerator String table = _dm.getTableName(); Statement stmt = conn.createStatement(); - try { - ResultSet rs = stmt.executeQuery( - " SELECT COUNT(*), MAX(" + liaison.columnSQL(column) + ") " + - " FROM " + liaison.tableSQL(table)); - if (!rs.next()) { - log.warning("Query on count()/max() bizarrely returned no rows."); - return null; - } - - int cnt = rs.getInt(1); - if (cnt > 0) { - return Integer.valueOf(rs.getInt(2)); - } + ResultSet rs = stmt.executeQuery( + " SELECT COUNT(*), MAX(" + liaison.columnSQL(column) + ") " + + " FROM " + liaison.tableSQL(table)); + if (!rs.next()) { + log.warning("Query on count()/max() bizarrely returned no rows."); return null; - - } finally { - JDBCUtil.close(stmt); } + + int cnt = rs.getInt(1); + if (cnt > 0) { + return Integer.valueOf(rs.getInt(2)); + } + return null; } public DepotMarshaller getDepotMarshaller ()