A whole bunch of revampery on the way to collection caching, including some

basic stats reporting and query and cache logging.

PersistenceContext no longer sticks its nose so deeply into the business of
Query and Modifier. It just passes itself along and allows them to do any cache
activity they need during their normal execution. The one situation where we
keep our nose in their business is to allow a Query to return a result from the
cache before it is even invoked so that we can avoid requesting a database
connection if we won't ever need one.

src/java/com/samskivert/depot/DepotMarshaller.java

@@ -578,7 +578,7 @@ public class DepotMarshaller<T extends PersistentRecord>
         // check to see if our schema version table exists, create it if not
         ctx.invoke(new Modifier() {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 liaison.createTableIfMissing(
                     conn, SCHEMA_VERSION_TABLE,
                     new String[] { P_COLUMN, V_COLUMN, MV_COLUMN },
@@ -697,7 +697,7 @@ public class DepotMarshaller<T extends PersistentRecord>
         final Iterable<Index> indexen = _indexes.values();
         ctx.invoke(new Modifier() {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 // create the table
                 String[] primaryKeyColumns = null;
                 if (_pkColumns != null) {
@@ -803,8 +803,8 @@ public class DepotMarshaller<T extends PersistentRecord>
         if (hasPrimaryKey() && metaData.pkName == null) {
             log.info("Adding primary key.");
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     liaison.addPrimaryKey(
                         conn, getTableName(), fieldsToColumns(getPrimaryKeyFields()));
                     return 0;
@@ -815,8 +815,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             final String pkName = metaData.pkName;
             log.info("Dropping primary key: " + pkName);
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     liaison.dropPrimaryKey(conn, getTableName(), pkName);
                     return 0;
                 }
@@ -833,8 +833,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             }
             // but this is a new, named index, so we create it
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     liaison.addIndexToTable(
                         conn, getTableName(), fieldsToColumns(index.fields()),
                         ixName, index.unique());
@@ -869,8 +869,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             final String[] colArr = colSet.toArray(new String[colSet.size()]);
             final String fName = indexName;
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     liaison.addIndexToTable(conn, getTableName(), colArr, fName, true);
                     return 0;
                 }
@@ -891,8 +891,8 @@ public class DepotMarshaller<T extends PersistentRecord>
 
             // but not this one, so let's create it
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     builder.addFullTextSearch(conn, DepotMarshaller.this, recordFts);
                     return 0;
                 }
@@ -937,8 +937,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             // initialValue which will use the new column name to obtain the sequence name which
             // ain't going to work either; we punt!
             ctx.invoke(new Modifier() {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     valgen.create(conn, liaison);
                     return 0;
                 }
@@ -983,7 +983,7 @@ public class DepotMarshaller<T extends PersistentRecord>
 
     protected abstract class SimpleModifier extends Modifier {
         @Override
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             Statement stmt = conn.createStatement();
             try {
                 return invoke(liaison, stmt);
@@ -1035,10 +1035,13 @@ public class DepotMarshaller<T extends PersistentRecord>
             throws DatabaseException
         {
             return ctx.invoke(new Query.Trivial<TableMetaData>() {
-                @Override public TableMetaData invoke (Connection conn, DatabaseLiaison dl)
+                @Override public TableMetaData invoke (PersistenceContext ctx, Connection conn,
-                    throws SQLException {
+                                                       DatabaseLiaison dl) throws SQLException {
                     return new TableMetaData(conn.getMetaData(), tableName);
                 }
+                public void updateStats (Stats stats) {
+                    // nothing doing
+                }
             });
         }
 

src/java/com/samskivert/depot/DepotRepository.java

@@ -37,6 +37,7 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
 import com.samskivert.util.ArrayUtil;
+import com.samskivert.util.StringUtil;
 
 import com.samskivert.jdbc.ConnectionProvider;
 import com.samskivert.jdbc.DatabaseLiaison;
@@ -308,7 +309,7 @@ public abstract class DepotRepository
      * the latest data.
      */
     protected <T extends PersistentRecord> List<Key<T>> findAllKeys (
-        Class<T> type, boolean forUpdate, Collection<? extends QueryClause> clauses)
+        final Class<T> type, boolean forUpdate, Collection<? extends QueryClause> clauses)
         throws DatabaseException
     {
         final List<Key<T>> keys = Lists.newArrayList();
@@ -319,36 +320,53 @@ public abstract class DepotRepository
 
         if (forUpdate) {
             _ctx.invoke(new Modifier(null) {
-                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
+                @Override
-                    throws SQLException {
+                protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                     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 " + StringUtil.shortClassName(type) + " keys",
+                                     "count", keys.size());
+                        }
                         return 0;
                     } finally {
                         JDBCUtil.close(stmt);
                     }
                 }
+                @Override public void updateStats (Stats stats) {
+                    stats.noteQuery(0, 1, 0, 0); // one uncached query
+                }
             });
 
         } else {
             _ctx.invoke(new Query.Trivial<Void>() {
                 @Override
-                public Void invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                public Void invoke (PersistenceContext ctx, Connection conn,
+                                    DatabaseLiaison liaison) throws SQLException {
                     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 " + StringUtil.shortClassName(type) + " keys",
+                                     "count", keys.size());
+                        }
                         return null;
                     } finally {
                         JDBCUtil.close(stmt);
                     }
                 }
+                public void updateStats (Stats stats) {
+                    stats.noteQuery(0, 1, 0, 0); // one uncached query
+                }
             });
         }
 
@@ -375,7 +393,7 @@ public abstract class DepotRepository
         // key will be null if record was supplied without a primary key
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 // if needed, update our modifier's key so that it can cache our results
                 Set<String> identityFields = Collections.emptySet();
                 if (_key == null) {
@@ -427,7 +445,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -464,7 +482,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 // clear out _result so that we don't rewrite this partial record to the cache
                 _result = null;
@@ -604,7 +622,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -735,7 +753,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -773,7 +791,7 @@ public abstract class DepotRepository
         final boolean[] created = new boolean[1];
         _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = null;
                 try {
                     if (_key != null) {
@@ -897,7 +915,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();

src/java/com/samskivert/depot/FindAllQuery.java

@@ -75,13 +75,12 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                 }
             }
 
-            SelectClause<T> select =
+            _select = new SelectClause<T>(_type, _marsh.getPrimaryKeyFields(), clauses);
-                new SelectClause<T>(_type, _marsh.getPrimaryKeyFields(), clauses);
+            _builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
-            _builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select));
+            _builder.newQuery(_select);
-            _builder.newQuery(select);
         }
 
-        public List<T> invoke (Connection conn, DatabaseLiaison liaison)
+        public List<T> invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
             throws SQLException
         {
             Map<Key<T>, T> entities = Maps.newHashMap();
@@ -96,15 +95,11 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                     Key<T> key = _marsh.makePrimaryKey(rs);
                     allKeys.add(key);
 
-                    // TODO: All this cache fiddling needs to move to PersistenceContext?
+                    T value = _ctx.<T>cacheLookup(key);
-                    CacheAdapter.CachedValue<T> hit = _ctx.cacheLookup(key);
+                    if (value != null) {
-                    if (hit != null) {
+                        @SuppressWarnings("unchecked") T newValue = (T) value.clone();
-                        T value = hit.getValue();
+                        entities.put(key, newValue);
-                        if (value != null) {
+                        continue;
-                            @SuppressWarnings("unchecked") T newValue = (T) value.clone();
-                            entities.put(key, newValue);
-                            continue;
-                        }
                     }
 
                     fetchKeys.add(key);
@@ -114,8 +109,23 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                 JDBCUtil.close(stmt);
             }
 
-            return loadAndResolve(conn, allKeys, fetchKeys, entities, stmtString);
+            _cachedRecords = entities.size();
+            _uncachedRecords = fetchKeys.size();
+
+            if (PersistenceContext.CACHE_DEBUG) {
+                log.info("Loaded " + _marsh.getTableName(), "query", _select,
+                         "keys", keysToString(allKeys));
+            }
+
+            return loadAndResolve(ctx, conn, allKeys, fetchKeys, entities, stmtString);
         }
+
+        public void updateStats (Stats stats) {
+            stats.noteQuery(0, 1, _cachedRecords, _uncachedRecords); // one uncached query
+        }
+
+        protected SelectClause<T> _select;
+        protected int _cachedRecords, _uncachedRecords;
     }
 
     /**
@@ -132,29 +142,33 @@ public abstract class FindAllQuery<T extends PersistentRecord>
             _builder = ctx.getSQLBuilder(new DepotTypes(ctx, _type));
         }
 
-        public List<T> invoke (Connection conn, DatabaseLiaison liaison)
+        public List<T> invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
             throws SQLException
         {
             Map<Key<T>, T> entities = Maps.newHashMap();
             Set<Key<T>> fetchKeys = Sets.newHashSet();
             for (Key<T> key : _keys) {
-                // TODO: All this cache fiddling needs to move to PersistenceContext?
+                T value = _ctx.<T>cacheLookup(key);
-                CacheAdapter.CachedValue<T> hit = _ctx.cacheLookup(key);
+                if (value != null) {
-                if (hit != null) {
+                    @SuppressWarnings("unchecked") T newValue = (T) value.clone();
-                    T value = hit.getValue();
+                    entities.put(key, newValue);
-                    if (value != null) {
+                    continue;
-                        @SuppressWarnings("unchecked") T newValue = (T) value.clone();
-                        entities.put(key, newValue);
-                        continue;
-                    }
                 }
                 fetchKeys.add(key);
             }
 
-            return loadAndResolve(conn, _keys, fetchKeys, entities, null);
+            _cachedRecords = entities.size();
+            _uncachedRecords = fetchKeys.size();
+
+            return loadAndResolve(ctx, conn, _keys, fetchKeys, entities, null);
+        }
+
+        public void updateStats (Stats stats) {
+            stats.noteQuery(0, 0, _cachedRecords, _uncachedRecords);
         }
 
         protected Collection<Key<T>> _keys;
+        protected int _cachedRecords, _uncachedRecords;
     }
 
     /**
@@ -168,12 +182,12 @@ public abstract class FindAllQuery<T extends PersistentRecord>
             throws DatabaseException
         {
             super(ctx, type);
-            SelectClause<T> select = new SelectClause<T>(type, _marsh.getFieldNames(), clauses);
+            _select = new SelectClause<T>(type, _marsh.getFieldNames(), clauses);
-            _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select));
+            _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
-            _builder.newQuery(select);
+            _builder.newQuery(_select);
         }
 
-        public List<T> invoke (Connection conn, DatabaseLiaison liaison)
+        public List<T> invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
             throws SQLException
         {
             List<T> result = Lists.newArrayList();
@@ -182,12 +196,25 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                 ResultSet rs = stmt.executeQuery();
                 while (rs.next()) {
                     result.add(_marsh.createObject(rs));
+                    _uncachedRecords++;
                 }
             } finally {
                 JDBCUtil.close(stmt);
             }
+            if (PersistenceContext.CACHE_DEBUG) {
+                log.info("Loaded " + _marsh.getTableName(), "query", _select,
+                         "uncached", _uncachedRecords);
+            }
+            // TODO: do we want to cache these results?
             return result;
         }
+
+        public void updateStats (Stats stats) {
+            stats.noteQuery(0, 1, 0, _uncachedRecords);
+        }
+
+        protected SelectClause<T> _select;
+        protected int _uncachedRecords;
     }
 
     public FindAllQuery (PersistenceContext ctx, Class<T> type)
@@ -198,45 +225,46 @@ public abstract class FindAllQuery<T extends PersistentRecord>
         _marsh = _ctx.getMarshaller(type);
     }
 
-    // from Query
+    // from interface Query
-    public CacheKey getCacheKey ()
+    public List<T> getCachedResult (PersistenceContext ctx)
     {
-        return null;
+        return null; // TODO
     }
 
-    // from Query
+//     // from interface Query
-    public void updateCache (PersistenceContext ctx, List<T> result) {
+//     public List<T> transformCacheHit (CacheKey key, List<T> bits)
-        if (_marsh.hasPrimaryKey()) {
+//     {
-            for (T bit : result) {
+//         if (bits == null) {
-                ctx.cacheStore(_marsh.getPrimaryKey(bit), bit.clone());
+//             return bits;
-            }
+//         }
-        }
-    }
 
-    // from Query
+//         List<T> result = Lists.newArrayList();
-    public List<T> transformCacheHit (CacheKey key, List<T> bits)
+//         for (T bit : bits) {
+//             if (bit != null) {
+//                 @SuppressWarnings("unchecked") T cbit = (T) bit.clone();
+//                 result.add(cbit);
+//             } else {
+//                 result.add(null);
+//             }
+//         }
+//         return result;
+//     }
+
+    // from interface Operation
+    public void updateStats (Stats stats)
     {
-        if (bits == null) {
+        // TODO
-            return bits;
-        }
-
-        List<T> result = Lists.newArrayList();
-        for (T bit : bits) {
-            if (bit != null) {
-                @SuppressWarnings("unchecked") T cbit = (T) bit.clone();
-                result.add(cbit);
-            } else {
-                result.add(null);
-            }
-        }
-        return result;
     }
 
-    protected List<T> loadAndResolve (Connection conn, Collection<Key<T>> allKeys,
+    protected List<T> loadAndResolve (PersistenceContext ctx, Connection conn,
-                                      Set<Key<T>> fetchKeys, Map<Key<T>, T> entities,
+                                      Collection<Key<T>> allKeys, Set<Key<T>> fetchKeys,
-                                      String origStmt)
+                                      Map<Key<T>, T> entities, String origStmt)
         throws SQLException
     {
+        if (PersistenceContext.CACHE_DEBUG && fetchKeys.size() > 0) {
+            log.info("Loading " + _marsh.getTableName(), "keys", keysToString(fetchKeys));
+        }
+
         // if we're fetching a huge number of records, we have to do it in multiple queries
         if (fetchKeys.size() > In.MAX_KEYS) {
             int keyCount = fetchKeys.size();
@@ -248,11 +276,11 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                     iter.remove();
                 }
                 keyCount -= keys.size();
-                loadRecords(conn, keys, entities, origStmt);
+                loadRecords(ctx, conn, keys, entities, origStmt);
             } while (keyCount > 0);
 
         } else if (fetchKeys.size() > 0) {
-            loadRecords(conn, fetchKeys, entities, origStmt);
+            loadRecords(ctx, conn, fetchKeys, entities, origStmt);
         }
 
         List<T> result = Lists.newArrayList();
@@ -265,10 +293,11 @@ public abstract class FindAllQuery<T extends PersistentRecord>
         return result;
     }
 
-    protected void loadRecords (Connection conn, Set<Key<T>> keys, Map<Key<T>, T> entities,
+    protected void loadRecords (PersistenceContext ctx, Connection conn, Set<Key<T>> keys,
-                                String origStmt)
+                                Map<Key<T>, T> entities, String origStmt)
         throws SQLException
     {
+        boolean hasPrimaryKey = _marsh.hasPrimaryKey();
         _builder.newQuery(new SelectClause<T>(_type, _marsh.getFieldNames(),
                                               new KeySet<T>(_type, keys)));
         PreparedStatement stmt = _builder.prepare(conn);
@@ -282,6 +311,10 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                 if (entities.put(key, obj) != null) {
                     dups++;
                 }
+                // cache our result if it has a primary key
+                if (hasPrimaryKey) {
+                    ctx.cacheStore(_marsh.getPrimaryKey(obj), obj.clone());
+                }
                 got.add(key);
                 cnt++;
             }
@@ -292,11 +325,27 @@ public abstract class FindAllQuery<T extends PersistentRecord>
                             "wanted", keys, "got", got, "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)
+    {
+        StringBuilder builder = new StringBuilder("(");
+        for (Key<T> key : keySet) {
+            if (builder.length() > 1) {
+                builder.append(", ");
+            }
+            key.toShortString(builder);
+        }
+        return builder.append(")").toString();
+    }
+
     protected PersistenceContext _ctx;
     protected SQLBuilder _builder;
     protected DepotMarshaller<T> _marsh;

src/java/com/samskivert/depot/FindOneQuery.java

@@ -27,9 +27,13 @@ import java.sql.SQLException;
 
 import com.samskivert.jdbc.DatabaseLiaison;
 import com.samskivert.jdbc.JDBCUtil;
+import com.samskivert.util.StringUtil;
+
 import com.samskivert.depot.clause.QueryClause;
 import com.samskivert.depot.clause.SelectClause;
 
+import static com.samskivert.depot.Log.log;
+
 /**
  * The implementation of {@link DepotRepository#load} functionality.
  */
@@ -49,21 +53,30 @@ public class FindOneQuery<T extends PersistentRecord>
         _builder.newQuery(_select);
     }
 
-    // from Query
+    // from interface Query
-    public CacheKey getCacheKey ()
+    public T getCachedResult (PersistenceContext ctx)
     {
-        WhereClause where = _select.getWhereClause();
+        CacheKey key = getCacheKey();
-        if (where != null && where instanceof CacheKey) {
+        if (key == null) {
-            return (CacheKey) where;
+            return null;
         }
-        return null;
+        T value = ctx.<T>cacheLookup(key);
+        if (value == null) {
+            return null;
+        }
+        _cachedRecords = 1;
+        // we do not want to return a reference to the actual cached entity so we clone it
+        @SuppressWarnings("unchecked") T cvalue = (T) value.clone();
+        return cvalue;
     }
 
-    // from Query
+    // from interface Query
-    public T invoke (Connection conn, DatabaseLiaison liaison) throws SQLException
+    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()) {
@@ -71,6 +84,25 @@ public class FindOneQuery<T extends PersistentRecord>
             }
             // 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 = _marsh.getPrimaryKey(result);
+                }
+            }
+            if (PersistenceContext.CACHE_DEBUG) {
+                log.info("Loaded " + (key != null ? key : _marsh.getTableName()));
+            }
+            if (key != null) {
+                ctx.cacheStore(key, (result != null) ? result.clone() : null);
+                if (PersistenceContext.CACHE_DEBUG) {
+                    log.info("Cached " + key);
+                }
+            }
+
             return result;
 
         } finally {
@@ -78,33 +110,20 @@ public class FindOneQuery<T extends PersistentRecord>
         }
     }
 
-    // from Query
+    // from interface Operation
-    public void updateCache (PersistenceContext ctx, T result)
+    public void updateStats (Stats stats)
     {
-        CacheKey key = getCacheKey();
+        stats.noteQuery(0, 0, _cachedRecords, 1-_cachedRecords);
-        if (key == null) {
-            // no row-specific cache key was given
-            if (result == null || !_marsh.hasPrimaryKey()) {
-                return;
-            }
-            // if we can, create a key from what was actually returned
-            key = _marsh.getPrimaryKey(result);
-        }
-        ctx.cacheStore(key, (result != null) ? result.clone() : null);
     }
 
-    // from Query
+    protected CacheKey getCacheKey ()
-    public T transformCacheHit (CacheKey key, T value)
     {
-        if (value == null) {
+        WhereClause where = _select.getWhereClause();
-            return null;
+        return (where != null && where instanceof CacheKey) ? (CacheKey)where : null;
-        }
-        // we do not want to return a reference to the actual cached entity so we clone it
-        @SuppressWarnings("unchecked") T cvalue = (T) value.clone();
-        return cvalue;
     }
 
     protected DepotMarshaller<T> _marsh;
     protected SelectClause<T> _select;
     protected SQLBuilder _builder;
+    protected int _cachedRecords;
 }

src/java/com/samskivert/depot/Key.java

@@ -199,6 +199,20 @@ public class Key<T extends PersistentRecord> extends WhereClause
         ctx.cacheInvalidate(this);
     }
 
+    /**
+     * Appends just the key=value portion of our {@link #toString} to the supplied buffer.
+     */
+    public void toShortString (StringBuilder builder)
+    {
+        String[] keyFields = KeyUtil.getKeyFields(_pClass);
+        for (int ii = 0; ii < keyFields.length; ii ++) {
+            if (ii > 0) {
+                builder.append(":");
+            }
+            builder.append(keyFields[ii]).append("=").append(_values.get(ii));
+        }
+    }
+
     @Override // from WhereClause
     public void validateQueryType (Class<?> pClass)
     {
@@ -229,13 +243,7 @@ public class Key<T extends PersistentRecord> extends WhereClause
     {
         StringBuilder builder = new StringBuilder(_pClass.getSimpleName());
         builder.append("(");
-        String[] keyFields = KeyUtil.getKeyFields(_pClass);
+        toShortString(builder);
-        for (int ii = 0; ii < keyFields.length; ii ++) {
-            if (ii > 0) {
-                builder.append(", ");
-            }
-            builder.append(keyFields[ii]).append("=").append(_values.get(ii));
-        }
         builder.append(")");
         return builder.toString();
     }

src/java/com/samskivert/depot/Modifier.java

@@ -3,7 +3,7 @@
 //
 // Depot library - a Java relational persistence library
 // Copyright (C) 2006-2008 Michael Bayne and Pär Winzell
-// 
+//
 // This library is free software; you can redistribute it and/or modify it
 // under the terms of the GNU Lesser General Public License as published
 // by the Free Software Foundation; either version 2.1 of the License, or
@@ -41,8 +41,8 @@ public abstract class Modifier implements Operation<Integer>
             super(null);
         }
 
-        @Override
+        @Override // from Modifier
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             Statement stmt = conn.createStatement();
             try {
                 return stmt.executeUpdate(createQuery(liaison));
@@ -86,25 +86,21 @@ public abstract class Modifier implements Operation<Integer>
         }
 
         @Override // from Modifier
-        public void cacheUpdate (PersistenceContext ctx)
+        public Integer invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
+            throws SQLException
         {
-            super.cacheUpdate(ctx);
+            Integer rows = super.invoke(ctx, conn, liaison);
             // if we have both a key and a record, cache
             if (_key != null && _result != null) {
                 ctx.cacheStore(_key, _result.clone());
             }
+            return rows;
         }
 
         protected CacheKey _key;
         protected T _result;
     }
 
-    /**
-     * Overriden to perform the actual database modifications represented by this object; should
-     * return the number of modified rows.
-     */
-    public abstract Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException;
-
     /**
      * Constructs a {@link Modifier} without a cache invalidator.
      */
@@ -121,24 +117,27 @@ public abstract class Modifier implements Operation<Integer>
         _invalidator = invalidator;
     }
 
-    /**
+    // from interface Operation
-     * Do any cache invalidation needed for this modification. This method is called just
+    public Integer invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
-     * before the database statement is executed.
+        throws SQLException
-     */
-    public void cacheInvalidation (PersistenceContext ctx)
     {
         if (_invalidator != null) {
             _invalidator.invalidate(ctx);
         }
+        return invoke(conn, liaison);
+    }
+
+    // from interface Operation
+    public void updateStats (Stats stats)
+    {
+        // nothing to update for modifiers
     }
 
     /**
-     * Do any cache updates needed for this modification. This method is called just after
+     * Overriden to perform the actual database modifications represented by this object; should
-     * the successful execution of the database statement.
+     * return the number of modified rows.
      */
-    public void cacheUpdate (PersistenceContext ctx)
+    protected abstract int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException;
-    {
-    }
 
     protected CacheInvalidator _invalidator;
 }

src/java/com/samskivert/depot/Operation.java

@@ -33,6 +33,11 @@ public interface Operation<T>
     /**
      * Performs the actual JDBC interactions associated with this operation.
      */
-    public T invoke (Connection conn, DatabaseLiaison liaison)
+    public T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
         throws SQLException;
+
+    /**
+     * Called after the operation has been invoked so that it can update our runtime statistics.
+     */
+    public void updateStats (Stats stats);
 }

src/java/com/samskivert/depot/PersistenceContext.java

@@ -53,6 +53,9 @@ public class PersistenceContext
     /** Allow toggling of query logging and other debug output via a system property. */
     public static final boolean DEBUG = Boolean.getBoolean("com.samskivert.depot.debug");
 
+    /** Allow toggling of cache-related logging via a system property. */
+    public static final boolean CACHE_DEBUG = Boolean.getBoolean("com.samskivert.depot.cache_debug");
+
     /** Map {@link TableGenerator} instances by name. */
     public Map<String, TableGenerator> tableGenerators = Maps.newHashMap();
 
@@ -173,6 +176,14 @@ public class PersistenceContext
         _conprov.shutdown();
     }
 
+    /**
+     * Returns a snapshot of our current runtime statistics.
+     */
+    public Stats.Snapshot getStats ()
+    {
+        return _stats.getSnapshot();
+    }
+
     /**
      * Create and return a new {@link SQLBuilder} for the appropriate dialect.
      *
@@ -258,26 +269,14 @@ public class PersistenceContext
     public <T> T invoke (Query<T> query)
         throws DatabaseException
     {
-        CacheKey key = query.getCacheKey();
+        // we check to see if the query is already cached before invoking it to avoid requesting a
-        // if there is a cache key, check the cache
+        // database connection if we don't actually need one
-        if (key != null && _cache != null) {
+        T result = query.getCachedResult(this);
-            CacheAdapter.CachedValue<T> cacheHit = cacheLookup(key);
+        if (result != null) {
-            if (cacheHit != null) {
+            query.updateStats(_stats);
-                log.debug("cache hit [key=" + key + ", hit=" + cacheHit + "]");
+            return result;
-                T value = cacheHit.getValue();
-                value = query.transformCacheHit(key, value);
-                if (value != null) {
-                    return value;
-                }
-                log.debug("transformCacheHit returned null; rejecting cached value.");
-            }
-            log.debug("cache miss [key=" + key + "]");
         }
-        // otherwise, perform the query
+        return invoke(query, true);
-        T result = invoke(query, true);
-        // and let the caller figure out if it wants to cache itself somehow
-        query.updateCache(this, result);
-        return result;
     }
 
     /**
@@ -286,12 +285,7 @@ public class PersistenceContext
     public int invoke (Modifier modifier)
         throws DatabaseException
     {
-        modifier.cacheInvalidation(this);
+        return invoke(modifier, true);
-        int rows = invoke(modifier, true);
-        if (rows > 0) {
-            modifier.cacheUpdate(this);
-        }
-        return rows;
     }
 
     /**
@@ -305,13 +299,14 @@ public class PersistenceContext
     /**
      * Looks up an entry in the cache by the given key.
      */
-    public <T> CacheAdapter.CachedValue<T> cacheLookup (CacheKey key)
+    public <T> T cacheLookup (CacheKey key)
     {
         if (_cache == null) {
             return null;
         }
         CacheAdapter.CacheBin<T> bin = _cache.getCache(key.getCacheId());
-        return bin.lookup(key.getCacheKey());
+        CacheAdapter.CachedValue<T> ref = bin.lookup(key.getCacheKey());
+        return (ref == null) ? null : ref.getValue();
     }
 
     /**
@@ -530,6 +525,7 @@ public class PersistenceContext
 
         boolean isReadOnly = !(op instanceof Modifier);
         Connection conn;
+        long preConnect = System.nanoTime();
         try {
             conn = _conprov.getConnection(_ident, isReadOnly);
         } catch (PersistenceException pe) {
@@ -541,10 +537,15 @@ public class PersistenceContext
         // connection pooling which will block in getConnection() instead of returning a connection
         // that someone else may be using
         synchronized (conn) {
+            long preInvoke = System.nanoTime();
             try {
-                // if this becomes more complex than this single statement, then this should turn
+                // invoke our database operation
-                // into a method call that contains the complexity
+                T value = op.invoke(this, conn, _liaison);
-                return op.invoke(conn, _liaison);
+                // note the time it took to invoke this operation
+                _stats.noteOp(isReadOnly, preConnect, preInvoke, System.nanoTime());
+                // have the operation update any appropriate runtime statistics as well
+                op.updateStats(_stats);
+                return value;
 
             } catch (SQLException sqe) {
                 if (!isReadOnly) {
@@ -594,6 +595,9 @@ public class PersistenceContext
     protected DatabaseLiaison _liaison;
     protected boolean _warnOnLazyInit;
 
+    /** Used to track various statistics. */
+    protected Stats _stats = new Stats();
+
     /** The object through which all our caching is relayed, or null, for no caching. */
     protected CacheAdapter _cache;
 

src/java/com/samskivert/depot/Query.java

@@ -34,40 +34,22 @@ public interface Query<T> extends Operation<T>
     /** A simple base class for non-complex queries. */
     public abstract class Trivial<T> implements Query<T>
     {
-        public abstract T invoke (Connection conn, DatabaseLiaison liaison) throws SQLException;
+        public abstract T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
+            throws SQLException;
 
-        public CacheKey getCacheKey () {
+        public T getCachedResult (PersistenceContext ctx) {
             return null;
         }
 
         public T transformCacheHit (CacheKey key, T value) {
             return value;
         }
-
-        public void updateCache (PersistenceContext ctx, T result) {
-        }
     }
 
     /**
-     * Any query may elect to utilize the built-in cache by returning a non-null {@link CacheKey}
+     * If this query has a simple cached result, it should return the non-null result from this
-     * in this method. This is done automatically by the {@link DepotRepository} when looking up
+     * method. If null is returned, the query will be {@link #invoke}d to obtain its result from
-     * single entities by primary key, but even entire collections can be cached under a single
+     * persistent storage.
-     * key.
-     *
-     * Great care must be taken to invalidate such cached collections when their constituent
-     * entities are invalidated. This is generally done using {@link CacheListener} and
-     * {@link CacheInvalidator}.
      */
-    public CacheKey getCacheKey ();
+    public T getCachedResult (PersistenceContext ctx);
-
-    /**
-     * Overriden by subclasses to perform special operations when the query would return a cache
-     * hit. The value may be mutated, modified, or null may be return to force a database hit.
-     */
-    public T transformCacheHit (CacheKey key, T value);
-
-    /**
-     * Overriden by subclasses to perform case-by-case cache updates.
-     */
-    public void updateCache (PersistenceContext ctx, T result);
 }

src/java/com/samskivert/depot/SchemaMigration.java

@@ -53,7 +53,7 @@ public abstract class SchemaMigration extends Modifier
         }
 
         @Override
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             if (!liaison.tableContainsColumn(conn, _tableName, _columnName)) {
                 // we'll accept this inconsistency
                 log.warning(_tableName + "." + _columnName + " already dropped.");
@@ -78,8 +78,12 @@ public abstract class SchemaMigration extends Modifier
             _newColumnName = newColumnName;
         }
 
+        @Override public boolean runBeforeDefault () {
+            return true;
+        }
+
         @Override
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             if (!liaison.tableContainsColumn(conn, _tableName, _oldColumnName)) {
                 if (liaison.tableContainsColumn(conn, _tableName, _newColumnName)) {
                     // we'll accept this inconsistency
@@ -104,10 +108,6 @@ public abstract class SchemaMigration extends Modifier
                 conn, _tableName, _oldColumnName, _newColumnName, _newColumnDef) ? 1 : 0;
         }
 
-        @Override public boolean runBeforeDefault () {
-            return true;
-        }
-
         @Override
         protected void init (String tableName, Map<String, FieldMarshaller<?>> marshallers) {
             super.init(tableName, marshallers);
@@ -135,18 +135,18 @@ public abstract class SchemaMigration extends Modifier
             _fieldName = fieldName;
         }
 
+        @Override public boolean runBeforeDefault () {
+            return false;
+        }
+
         @Override
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             log.info("Updating type of '" + _fieldName + "' in " + _tableName);
             return liaison.changeColumn(conn, _tableName, _fieldName, _newColumnDef.getType(),
                 _newColumnDef.isNullable(), _newColumnDef.isUnique(),
                 _newColumnDef.getDefaultValue()) ? 1 : 0;
         }
 
-        @Override public boolean runBeforeDefault () {
-            return false;
-        }
-
         @Override
         protected void init (String tableName, Map<String, FieldMarshaller<?>> marshallers) {
             super.init(tableName, marshallers);
@@ -174,8 +174,12 @@ public abstract class SchemaMigration extends Modifier
             _defaultValue = defaultValue;
         }
 
+        @Override public boolean runBeforeDefault () {
+            return true;
+        }
+
         @Override
-        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             // override the default value in the column definition with the one provided
             ColumnDefinition defColumnDef = new ColumnDefinition(
                 _newColumnDef.getType(), _newColumnDef.isNullable(),
@@ -190,10 +194,6 @@ public abstract class SchemaMigration extends Modifier
             return 0;
         }
 
-        @Override public boolean runBeforeDefault () {
-            return true;
-        }
-
         @Override
         protected void init (String tableName, Map<String, FieldMarshaller<?>> marshallers) {
             super.init(tableName, marshallers);

src/java/com/samskivert/depot/Stats.java

@@ -0,0 +1,138 @@
+//
+// $Id$
+//
+// Depot library - a Java relational persistence library
+// Copyright (C) 2006-2008 Michael Bayne and Pär Winzell
+// 
+// This library is free software; you can redistribute it and/or modify it
+// under the terms of the GNU Lesser General Public License as published
+// by the Free Software Foundation; either version 2.1 of the License, or
+// (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+package com.samskivert.depot;
+
+import com.samskivert.util.Histogram;
+
+import static com.samskivert.depot.Log.log;
+
+/**
+ * Manages Depot performance statistics.
+ */
+public class Stats
+{
+    /**
+     * An immutable class used to report statistics on repository activity. Statistics are tracked
+     * from the start of the VM and are never reset.
+     */
+    public static class Snapshot
+    {
+        /** The total number of queries and modifiers executed. */
+        public final int totalOps;
+
+        /** The total number of milliseconds spent waiting for a JDBC connection. */
+        public final long connectionWaitTime;
+
+        /** The total number of collection queries that were loaded from the cache. */
+        public final int cachedQueries;
+
+        /** The total number of collection queries that were loaded from the database. */
+        public final int uncachedQueries;
+
+        /** The number of record loads (individual or as part of a collection query) that were
+         * loaded from the cache. */
+        public final long cachedRecords;
+
+        /** The number of record loads (individual or as part of a collection query) that were
+         * loaded from the database. */
+        public final long uncachedRecords;
+
+        /** A histogram of query durations (with 500ms buckets). */
+        public final Histogram queryHisto;
+
+        /** The total number of milliseconds spent executing queries. */
+        public final long queryTime;
+
+        /** A histogram of modifier durations (with 500ms buckets). */
+        public final Histogram modifierHisto;
+
+        /** The total number of milliseconds spent executing modifiers. */
+        public final long modifierTime;
+
+        /** Creates a stats instance. */
+        protected Snapshot (int totalOps, long connectionWaitTime,
+                            int cachedQueries, int uncachedQueries,
+                            int cachedRecords, int uncachedRecords,
+                            Histogram queryHisto, long queryTime,
+                            Histogram modifierHisto, long modifierTime)
+        {
+            this.totalOps = totalOps;
+            this.connectionWaitTime = connectionWaitTime;
+            this.cachedQueries = cachedQueries;
+            this.uncachedQueries = uncachedQueries;
+            this.cachedRecords = cachedRecords;
+            this.uncachedRecords = uncachedRecords;
+            this.queryHisto = queryHisto;
+            this.queryTime = queryTime;
+            this.modifierHisto = modifierHisto;
+            this.modifierTime = modifierTime;
+        }
+    }
+
+    public synchronized Snapshot getSnapshot ()
+    {
+        return new Snapshot(_totalOps, _connectionWaitTime,
+                            _cachedQueries, _uncachedQueries, _cachedRecords, _uncachedRecords,
+                            _readHisto.clone(), _readTime, _writeHisto.clone(), _writeTime);
+    }
+
+    public synchronized void noteOp (
+        boolean isReadOnly, long preConnect, long preInvoke, long postInvoke)
+    {
+        _totalOps++;
+        _connectionWaitTime += (preInvoke - preConnect) / 1000L;
+
+        long opTime = (postInvoke - preInvoke) / 1000L;
+        if (opTime > Integer.MAX_VALUE) {
+            log.warning("ZOMG! A database operation took " + opTime + "ms to complete!");
+            opTime = Integer.MAX_VALUE;
+        }
+
+        if (isReadOnly) {
+            _readTime += opTime;
+            _readHisto.addValue((int)opTime);
+        } else {
+            _writeTime += opTime;
+            _writeHisto.addValue((int)opTime);
+        }
+    }
+
+    public synchronized void noteQuery (int cachedQueries, int uncachedQueries,
+                                        int cachedRecords, int uncachedRecords)
+    {
+        _cachedQueries += cachedQueries;
+        _uncachedQueries += uncachedQueries;
+        _cachedRecords += cachedRecords;
+        _uncachedRecords += uncachedRecords;
+    }
+
+    protected int _totalOps;
+    protected long _connectionWaitTime;
+
+    protected Histogram _readHisto = new Histogram(0, 500, 20);
+    protected long _readTime;
+
+    protected Histogram _writeHisto = new Histogram(0, 500, 20);
+    protected long _writeTime;
+
+    protected int _cachedQueries, _uncachedQueries;
+    protected int _cachedRecords, _uncachedRecords;
+}

src/java/com/samskivert/depot/clause/SelectClause.java

@@ -195,6 +195,32 @@ public class SelectClause<T extends PersistentRecord> extends QueryClause
         builder.visit(this);
     }
 
+    @Override // from Object
+    public String toString ()
+    {
+        StringBuilder builder = new StringBuilder();
+        builder.append("(where=").append(_where);
+        if (_fromOverride != null) {
+            builder.append(", from=").append(_fromOverride);
+        }
+        if (!_joinClauses.isEmpty()) {
+            builder.append(", join=").append(_joinClauses);
+        }
+        if (_orderBy != null) {
+            builder.append(", orderBy=").append(_orderBy);
+        }
+        if (_groupBy != null) {
+            builder.append(", groupBy=").append(_groupBy);
+        }
+        if (_limit != null) {
+            builder.append(", limit=").append(_limit);
+        }
+        if (_forUpdate != null) {
+            builder.append(", forUpdate=").append(_forUpdate);
+        }
+        return builder.append(")").toString();
+    }
+
     /** Persistent class fields mapped to field override clauses. */
     protected Map<String, FieldDefinition> _disMap = Maps.newHashMap();
 

src/java/com/samskivert/depot/clause/Where.java

@@ -85,6 +85,12 @@ public class Where extends WhereClause
         _condition.addClasses(classSet);
     }
 
+    @Override // from Object
+    public String toString ()
+    {
+        return String.valueOf(_condition);
+    }
+
     protected static SQLExpression toCondition (ColumnExp[] columns, Comparable<?>[] values)
     {
         SQLExpression[] comparisons = new SQLExpression[columns.length];

src/java/com/samskivert/depot/expression/ColumnExp.java

@@ -59,6 +59,12 @@ public class ColumnExp
         return _pField;
     }
 
+    @Override // from Object
+    public String toString ()
+    {
+        return "\"" + _pField + "\""; // TODO: qualify with record name and be uber verbose?
+    }
+
     /** The table that hosts the column we reference, or null. */
     protected final Class<? extends PersistentRecord> _pClass;
 

src/java/com/samskivert/depot/expression/ValueExp.java

@@ -51,6 +51,12 @@ public class ValueExp
         return _value;
     }
 
+    @Override // from Object
+    public String toString ()
+    {
+        return (_value instanceof Number) ? String.valueOf(_value) : ("'" + _value + "'");
+    }
+
     /** The value to be bound to the SQL parameters. */
     protected Object _value;
 }

src/java/com/samskivert/depot/operator/Conditionals.java

@@ -69,6 +69,12 @@ public abstract class Conditionals
         {
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            return "IsNull(" + _column + ")";
+        }
+
         protected ColumnExp _column;
     }
 
@@ -240,6 +246,20 @@ public abstract class Conditionals
             _column.addClasses(classSet);
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            StringBuilder builder = new StringBuilder();
+            builder.append(_column).append(" in (");
+            for (int ii = 0; ii < _values.length; ii++) {
+                if (ii > 0) {
+                    builder.append(", ");
+                }
+                builder.append(_values[ii]);
+            }
+            return builder.append(")").toString();
+        }
+
         protected ColumnExp _column;
         protected Comparable<?>[] _values;
     }
@@ -286,6 +306,12 @@ public abstract class Conditionals
             return _clause;
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            return "Exists(" + _clause + ")";
+        }
+
         protected SelectClause<T> _clause;
     }
 
@@ -329,6 +355,12 @@ public abstract class Conditionals
         {
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            return "FullText(" + _name + "=" + _query + ")";
+        }
+
         protected Class<? extends PersistentRecord> _pClass;
         protected String _name;
         protected String _query;

src/java/com/samskivert/depot/operator/Logic.java

@@ -49,7 +49,7 @@ public abstract class Logic
 
         @Override public String operator()
         {
-            return "or";
+            return " or ";
         }
     }
 
@@ -70,7 +70,7 @@ public abstract class Logic
 
         @Override public String operator()
         {
-            return "and";
+            return " and ";
         }
     }
 
@@ -102,6 +102,12 @@ public abstract class Logic
             _condition.addClasses(classSet);
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            return "Not(" + _condition + ")";
+        }
+
         protected SQLExpression _condition;
     }
 }

src/java/com/samskivert/depot/operator/SQLOperator.java

@@ -69,6 +69,19 @@ public interface SQLOperator extends SQLExpression
          */
         public abstract String operator ();
 
+        @Override // from Object
+        public String toString ()
+        {
+            StringBuilder builder = new StringBuilder("(");
+            for (SQLExpression condition : _conditions) {
+                if (builder.length() > 1) {
+                    builder.append(operator());
+                }
+                builder.append(condition);
+            }
+            return builder.append(")").toString();
+        }
+
         protected SQLExpression[] _conditions;
     }
 
@@ -116,6 +129,12 @@ public interface SQLOperator extends SQLExpression
             return _rhs;
         }
 
+        @Override // from Object
+        public String toString ()
+        {
+            return "(" + _lhs + operator() + _rhs + ")";
+        }
+
         protected SQLExpression _lhs;
         protected SQLExpression _rhs;
     }