A (slightly modified) patch from Charlie to use the magical statement

collecting Connection to automatically close all statements created during the
course of our operations.

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);
+                int mods = builder.prepare(conn).executeUpdate();
-                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);
-                }
             }
         });
     }
@@ -698,22 +689,19 @@ public abstract class DepotRepository
         _ctx.invoke(new CachingModifier<T>(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 = builder.prepare(conn).executeUpdate();
-                        int mods = stmt.executeUpdate();
                     if (mods > 0) {
                         // if it succeeded, we're done
                         return mods;
                     }
-                        JDBCUtil.close(stmt);
                 }
 
                 // if the update modified zero rows or the primary key was unset, insert
                 Set<String> 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
@@ -722,8 +710,7 @@ public abstract class DepotRepository
 
                 builder.newQuery(new InsertClause(pClass, _result, identityFields));
 
-                    stmt = builder.prepare(conn);
+                int mods = builder.prepare(conn).executeUpdate();
-                    int mods = stmt.executeUpdate();
 
                 // run any post-factum value generators and potentially generate our key
                 if (_key == null) {
@@ -732,10 +719,6 @@ public abstract class DepotRepository
                 }
                 created[0] = true;
                 return mods;
-
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
             }
         });
         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);
+                return builder.prepare(conn).executeUpdate();
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
             }
         });
     }
@@ -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);
+                return builder.prepare(conn).executeUpdate();
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
             }
         });
     }

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<Statement> 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();

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<T extends PersistentRecord>
         throws SQLException
     {
         if (builder.newQuery(clause)) {
-            PreparedStatement stmt = builder.prepare(conn);
+            return builder.prepare(conn).executeUpdate();
-            try {
-                return stmt.executeUpdate();
-            } finally {
-                JDBCUtil.close(stmt);
-            }
         }
         return 0;
     }

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,7 +74,6 @@ public class FindAllKeysQuery<T extends PersistentRecord> extends Query<XList<Ke
     {
         XList<Key<T>> keys = new XArrayList<Key<T>>();
         PreparedStatement stmt = _builder.prepare(conn);
-        try {
         ResultSet rs = stmt.executeQuery();
         while (rs.next()) {
             keys.add(_marsh.makePrimaryKey(rs));
@@ -86,9 +83,6 @@ public class FindAllKeysQuery<T extends PersistentRecord> extends Query<XList<Ke
             log.info("Loaded " + _marsh.getTableName(), "count", keys.size());
         }
         return keys;
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     // from Query

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,14 +130,10 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<XLi
                 builder.newQuery(_select);
                 PreparedStatement stmt = builder.prepare(conn);
                 stmtString = stmt.toString(); // for debugging
-                try {
                 ResultSet rs = stmt.executeQuery();
                 while (rs.next()) {
                     keys.add(_marsh.makePrimaryKey(rs));
                 }
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
                 _keys = KeySet.newKeySet(_type, keys);
                 _uncachedQueries++;
                 if (PersistenceContext.CACHE_DEBUG) {
@@ -239,15 +233,10 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<XLi
             XList<T> result = new XArrayList<T>();
             SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
             builder.newQuery(_select);
-            PreparedStatement stmt = builder.prepare(conn);
+            ResultSet rs = builder.prepare(conn).executeQuery();
-            try {
-                ResultSet rs = stmt.executeQuery();
             while (rs.next()) {
                 result.add(_marsh.createObject(rs));
             }
-            } finally {
-                JDBCUtil.close(stmt);
-            }
             _explicitQueries++;
             if (PersistenceContext.CACHE_DEBUG) {
                 log.info("Loaded " + _marsh.getTableName(), "query", _select, "rows",
@@ -348,10 +337,8 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<XLi
             _type, _marsh.getFieldNames(), KeySet.newKeySet(_type, keys));
         SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select));
         builder.newQuery(select);
-        PreparedStatement stmt = builder.prepare(conn);
-        try {
         Set<Key<T>> got = Sets.newHashSet();
-            ResultSet rs = stmt.executeQuery();
+        ResultSet rs = builder.prepare(conn).executeQuery();
         int cnt = 0, dups = 0;
         while (rs.next()) {
             T obj = _marsh.createObject(rs);
@@ -369,18 +356,14 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<XLi
             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(),
+                "wanted", KeySet.newKeySet(_type, keys).toString(), "got", KeySet.newKeySet(
-                            "got", KeySet.newKeySet(_type, got).toString(),
+                    _type, got).toString(), "dups", dups, new Exception());
-                            "dups", dups, new Exception());
         }
 
         if (PersistenceContext.CACHE_DEBUG) {
             log.info("Cached " + _marsh.getTableName(), "count", cnt);
         }
 
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     protected String keysToString (Iterable<Key<T>> keySet)

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,11 +79,9 @@ public class FindOneQuery<T extends PersistentRecord> extends Query<T>
     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();
+        ResultSet rs = _builder.prepare(conn).executeQuery();
         if (rs.next()) {
             result = _marsh.createObject(rs);
         }
@@ -112,10 +107,6 @@ public class FindOneQuery<T extends PersistentRecord> extends Query<T>
         }
 
         return result;
-
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     // from Operation

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());
+        conn.createStatement().executeUpdate(update.toString());
-        } finally {
-            JDBCUtil.close(stmt);
-        }
         return true;
     }
 

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<T>
     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;

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 + ", " +
-            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);
-        }
         return true;
     }
 

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,9 +68,7 @@ public class TableValueGenerator extends ValueGenerator
             new String[] { _pkColumnName });
 
         // and also that there's a row in it for us
-        PreparedStatement stmt = null;
+        PreparedStatement stmt = conn.prepareStatement(
-        try {
-            stmt = conn.prepareStatement(
             " SELECT * FROM " + liaison.tableSQL(_valueTable) +
             "  WHERE " + liaison.columnSQL(_pkColumnName) + " = ?");
         stmt.setString(1, _pkColumnValue);
@@ -79,9 +76,6 @@ public class TableValueGenerator extends ValueGenerator
             return;
         }
 
-            JDBCUtil.close(stmt);
-            stmt = null;
-
         int initialValue = _initialValue;
         if (_migrateIfExists) {
             Integer max = getFieldMaximum(conn, liaison);
@@ -97,43 +91,30 @@ public class TableValueGenerator extends ValueGenerator
         stmt.setString(1, _pkColumnValue);
         stmt.setInt(2, initialValue);
         stmt.executeUpdate();
-
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     @Override // from ValueGenerator
     public void delete (Connection conn, DatabaseLiaison liaison) throws SQLException
     {
-        PreparedStatement stmt = null;
+        PreparedStatement stmt = conn.prepareStatement(
-        try {
-            stmt = conn.prepareStatement(
             " DELETE FROM " + liaison.tableSQL(_valueTable) +
             "       WHERE " + liaison.columnSQL(_pkColumnName) + " = ?");
         stmt.setString(1, _pkColumnValue);
         stmt.executeUpdate();
-
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     @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(
+        PreparedStatement readStatement = conn.prepareStatement(
             " SELECT " + liaison.columnSQL(_valueColumnName) +
             "   FROM " + liaison.tableSQL(_valueTable) +
             "  WHERE " + liaison.columnSQL(_pkColumnName) + " = ? ");
         readStatement.setString(1, _pkColumnValue);
 
-            writeStatement = conn.prepareStatement(
+        PreparedStatement writeStatement = conn.prepareStatement(
             " UPDATE " + liaison.tableSQL(_valueTable) +
             "    SET " + liaison.columnSQL(_valueColumnName) + " = ? " +
             "  WHERE " + liaison.columnSQL(_pkColumnName) + " = ? " +
@@ -164,11 +145,6 @@ public class TableValueGenerator extends ValueGenerator
         throw new SQLException(
             "Failed to claim next primary key value in 10 attempts [table=" + _valueTable +
             ", column=" + _valueColumnName + "]");
-
-        } finally {
-            JDBCUtil.close(readStatement);
-            JDBCUtil.close(writeStatement);
-        }
     }
 
     /**

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,7 +84,6 @@ 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));
@@ -99,10 +97,6 @@ public abstract class ValueGenerator
             return Integer.valueOf(rs.getInt(2));
         }
         return null;
-
-        } finally {
-            JDBCUtil.close(stmt);
-        }
     }
 
     public DepotMarshaller<?> getDepotMarshaller ()