Match the changes in com.samskivert.jdbc.* by ridding ourselves of static column defining SQL strings. Also give ValueGenerators an option to calculate their initial value based on MAX() of the column for which values are being generated. As part of this we also make sure to initialize new ValueGenerators after all migrations take place, and add a way for them to clean up after themselves as well.

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

@@ -49,6 +49,7 @@ import com.samskivert.jdbc.depot.annotation.TableGenerator;
 import com.samskivert.jdbc.depot.annotation.Transient;
 import com.samskivert.jdbc.depot.annotation.UniqueConstraint;
 
+import com.samskivert.jdbc.ColumnDefinition;
 import com.samskivert.jdbc.DatabaseLiaison;
 import com.samskivert.util.ArrayUtil;
 
@@ -539,12 +540,12 @@ public class DepotMarshaller<T extends PersistentRecord>
         // figure out the list of fields that correspond to actual table columns and generate the
         // SQL used to create and migrate our table (unless we're a computed entity)
         _columnFields = new String[_allFields.length];
-        String[] declarations = new String[_allFields.length];
+        ColumnDefinition[] declarations = new ColumnDefinition[_allFields.length];
         int jj = 0;
         for (int ii = 0; ii < _allFields.length; ii++) {
             FieldMarshaller fm = _fields.get(_allFields[ii]);
             // include all persistent non-computed fields
-            String colDef = fm.getColumnDefinition();
+            ColumnDefinition colDef = fm.getColumnDefinition();
             if (colDef != null) {
                 _columnFields[jj] = _allFields[ii];
                 declarations[jj] = colDef;
@@ -566,7 +567,10 @@ public class DepotMarshaller<T extends PersistentRecord>
                 liaison.createTableIfMissing(
                     conn, SCHEMA_VERSION_TABLE,
                     new String[] { "persistentClass", "version" },
-                    new String[] { "VARCHAR(255) NOT NULL", "INTEGER NOT NULL" },
+                    new ColumnDefinition[] {
+                        new ColumnDefinition("VARCHAR(255)", false, true, null),
+                        new ColumnDefinition("INTEGER", false, false, null)
+                    },
                     null,
                     new String[] { "persistentClass" });
                 return 0;
@@ -578,7 +582,7 @@ public class DepotMarshaller<T extends PersistentRecord>
 
         // if the table does not exist, create it
         if (!metaData.tableExists) {
-            final String[] fDeclarations = declarations;
+            final ColumnDefinition[] fDeclarations = declarations;
             final String[][] uniqueConCols = new String[_uniqueConstraints.size()][];
             int kk = 0;
             for (Set<String> colSet : _uniqueConstraints) {
@@ -606,7 +610,7 @@ public class DepotMarshaller<T extends PersistentRecord>
                             getTableName() + "_" + idx.name(), idx.unique());
                     }
 
-                    // its value generators
+                    // initialize our value generators
                     for (ValueGenerator vg : _valueGenerators.values()) {
                         vg.init(conn, liaison);
                     }
@@ -684,13 +688,13 @@ public class DepotMarshaller<T extends PersistentRecord>
             }
 
             // otherwise add the column
-            final String coldef = fmarsh.getColumnDefinition();
+            final ColumnDefinition coldef = fmarsh.getColumnDefinition();
-            log.info("Adding column to " + getTableName() + ": " +
+            log.info("Adding column to " + getTableName() + ": " + fmarsh.getColumnName());
-                     fmarsh.getColumnName() + " " + coldef);
             ctx.invoke(new Modifier.Simple() {
                 protected String createQuery (DatabaseLiaison liaison) {
                     return "alter table " + liaison.tableSQL(getTableName()) +
-                        " add column " + liaison.columnSQL(fmarsh.getColumnName()) + " " + coldef;
+                        " add column " + liaison.columnSQL(fmarsh.getColumnName()) + " " +
+                        liaison.expandDefinition(coldef);
                 }
             });
 
@@ -699,8 +703,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             // TIMESTAMP columns a value of "0000-00-00 00:00:00" regardless of whether we
             // explicitly provide a "DEFAULT" value for the column or not, and DATETIME columns
             // cannot accept CURRENT_TIME or NOW() defaults at all.
-            if (coldef.toLowerCase().indexOf(" timestamp") != -1 ||
+            if (coldef.getType().equalsIgnoreCase("timestamp") ||
-                coldef.toLowerCase().indexOf(" datetime") != -1) {
+                coldef.getType().equalsIgnoreCase("datetime")) {
                 log.info("Assigning current time to " + fmarsh.getColumnName() + ".");
                 ctx.invoke(new Modifier.Simple() {
                     protected String createQuery (DatabaseLiaison liaison) {
@@ -823,6 +827,18 @@ public class DepotMarshaller<T extends PersistentRecord>
             }
         }
 
+        // last of all (re-)initialize our value generators, since one might've been added
+        if (_valueGenerators.size() > 0) {
+            ctx.invoke(new Modifier() {
+                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    for (ValueGenerator vg : _valueGenerators.values()) {
+                        vg.init(conn, liaison);
+                    }
+                    return 0;
+                }
+            });
+        }
+
         // record our new version in the database
         ctx.invoke(new Modifier() {
             public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {

src/java/com/samskivert/jdbc/depot/EntityMigration.java

@@ -24,6 +24,7 @@ import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.Map;
 
+import com.samskivert.jdbc.ColumnDefinition;
 import com.samskivert.jdbc.DatabaseLiaison;
 
 import static com.samskivert.jdbc.depot.Log.log;
@@ -105,7 +106,8 @@ public abstract class EntityMigration extends Modifier
             _newColumnDef = marshallers.get(_newColumnName).getColumnDefinition();
         }
 
-        protected String _oldColumnName,  _newColumnName, _newColumnDef;
+        protected String _oldColumnName,  _newColumnName;
+        protected ColumnDefinition _newColumnDef;
     }
 
     /**
@@ -122,7 +124,9 @@ public abstract class EntityMigration extends Modifier
 
         public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             log.info("Updating type of '" + _fieldName + "' in " + _tableName);
-            return liaison.changeColumn(conn, _tableName, _fieldName, _newColumnDef) ? 1 : 0;
+            return liaison.changeColumn(conn, _tableName, _fieldName, _newColumnDef.getType(),
+                _newColumnDef.isNullable(), _newColumnDef.isUnique(),
+                _newColumnDef.getDefaultValue()) ? 1 : 0;
         }
 
         public boolean runBeforeDefault () {
@@ -135,7 +139,8 @@ public abstract class EntityMigration extends Modifier
             _newColumnDef = marshallers.get(_fieldName).getColumnDefinition();
         }
 
-        protected String _fieldName, _columnName, _newColumnDef;
+        protected String _fieldName, _columnName;
+        protected ColumnDefinition _newColumnDef;
     }
 
     /**

src/java/com/samskivert/jdbc/depot/FieldMarshaller.java

@@ -34,10 +34,10 @@ import java.sql.Time;
 import java.sql.Timestamp;
 
 import com.samskivert.io.PersistenceException;
+import com.samskivert.jdbc.ColumnDefinition;
 import com.samskivert.jdbc.depot.annotation.Column;
 import com.samskivert.jdbc.depot.annotation.Computed;
 import com.samskivert.jdbc.depot.annotation.GeneratedValue;
-import com.samskivert.jdbc.depot.annotation.Id;
 
 import com.samskivert.util.StringUtil;
 
@@ -156,7 +156,7 @@ public abstract class FieldMarshaller<T>
     /**
      * Returns the SQL used to define this field's column.
      */
-    public String getColumnDefinition ()
+    public ColumnDefinition getColumnDefinition ()
     {
         return _columnDefinition;
     }
@@ -458,7 +458,8 @@ public abstract class FieldMarshaller<T>
     }
 
     protected Field _field;
-    protected String _columnName, _columnDefinition;
+    protected String _columnName;
+    ColumnDefinition _columnDefinition;
     protected Computed _computed;
     protected GeneratedValue _generatedValue;
 }

src/java/com/samskivert/jdbc/depot/IdentityValueGenerator.java

@@ -36,13 +36,13 @@ public class IdentityValueGenerator extends ValueGenerator
         super(gv, dm, fm);
     }
 
-    // from interface KeyGenerator
+    @Override // from ValueGenerator
     public boolean isPostFactum ()
     {
         return true;
     }
 
-    // from interface KeyGenerator
+    @Override // from ValueGenerator
     public void init (Connection conn, DatabaseLiaison liaison)
         throws SQLException
     {
@@ -50,10 +50,17 @@ public class IdentityValueGenerator extends ValueGenerator
             conn, _dm.getTableName(), _fm.getColumnName(), _initialValue, _allocationSize);
     }
 
-    // from interface KeyGenerator
+    @Override // from ValueGenerator
     public int nextGeneratedValue (Connection conn, DatabaseLiaison liaison)
         throws SQLException
     {
         return liaison.lastInsertedId(conn, _dm.getTableName(), _fm.getColumnName());
     }
+
+    @Override // from ValueGenerator
+    public void delete (Connection conn, DatabaseLiaison liaison)
+        throws SQLException
+    {
+        liaison.deleteGenerator(conn, _dm.getTableName(), _fm.getColumnName());
+    }
 }

src/java/com/samskivert/jdbc/depot/SQLBuilder.java

@@ -27,7 +27,7 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 
-import com.samskivert.jdbc.depot.DepotMarshaller.TableMetaData;
+import com.samskivert.jdbc.ColumnDefinition;
 import com.samskivert.jdbc.depot.annotation.Column;
 import com.samskivert.jdbc.depot.annotation.FullTextIndex;
 import com.samskivert.jdbc.depot.annotation.GeneratedValue;
@@ -91,6 +91,10 @@ public abstract class SQLBuilder
         return stmt;
     }
 
+    protected String nullify (String str) {
+        return (str != null && str.length() > 0) ? str : null;
+    }
+
     /**
      * Generates the SQL needed to construct a database column for field represented by the given
      * {@link FieldMarshaller}.
@@ -98,7 +102,7 @@ public abstract class SQLBuilder
      * TODO: This method should be split into several parts that are more easily overridden on a
      * case-by-case basis in the dialectal subclasses.
      */
-    public String buildColumnDefinition (FieldMarshaller fm)
+    public ColumnDefinition buildColumnDefinition (FieldMarshaller fm)
     {
         // if this field is @Computed, it has no SQL definition
         if (fm.getComputed() != null) {
@@ -107,34 +111,29 @@ public abstract class SQLBuilder
 
         Field field = fm.getField();
 
-        // read our column metadata from the annotation (if it exists); annoyingly we can't create
+        String type = null;
-        // a Column instance to read the defaults so we have to duplicate them here
-        int length = 255;
         boolean nullable = false;
         boolean unique = false;
-        String type = "";
+        String defaultValue = null;
-        String defval = "";
+        int length = 255;
+
         Column column = field.getAnnotation(Column.class);
         if (column != null) {
+            type = nullify(column.type());
             nullable = column.nullable();
             unique = column.unique();
+            defaultValue = nullify(column.defaultValue());
             length = column.length();
-            type = column.type();
-            defval = column.defaultValue();
         }
 
-        // create our SQL column definition
-        StringBuilder builder = new StringBuilder();
-        boolean typeDone = false;
-
         // handle primary keyness
         GeneratedValue genValue = fm.getGeneratedValue();
         if (genValue != null) {
             switch (genValue.strategy()) {
             case AUTO:
             case IDENTITY:
-                builder.append(" SERIAL UNIQUE");
+                type = "SERIAL";
-                typeDone = true;
+                unique = true;
                 break;
             case SEQUENCE: // TODO
                 throw new IllegalArgumentException(
@@ -145,48 +144,33 @@ public abstract class SQLBuilder
             }
         }
 
-        if (!typeDone) {
+        if (type == null) {
-            if (type.length() == 0) {
+            type = getColumnType(fm, length);
-                type = getColumnType(fm, length);
+        }
-            }
-            builder.append(" ").append(type);
 
-            // TODO: handle precision and scale
+        // sanity check nullability
+        if (nullable && field.getType().isPrimitive()) {
+            throw new IllegalArgumentException(
+                "Primitive Java type cannot be nullable [field=" + field.getName() + "]");
+        }
 
-            // sanity check nullability
+        if (defaultValue == null) {
-            if (nullable && field.getType().isPrimitive()) {
+            if (field.getType().equals(Byte.TYPE) ||
-                throw new IllegalArgumentException(
+                    field.getType().equals(Short.TYPE) ||
-                    "Primitive Java type cannot be nullable [field=" + field.getName() + "]");
+                    field.getType().equals(Integer.TYPE) ||
-            }
+                    field.getType().equals(Long.TYPE) ||
-
+                    field.getType().equals(Float.TYPE) ||
-            // handle nullability and uniqueness
+                    field.getType().equals(Double.TYPE)) {
-            if (!nullable) {
-                builder.append(" NOT NULL");
-            }
-            if (unique) {
-                builder.append(" UNIQUE");
-            }
-
-            // append the default value if one was specified
-            if (defval.length() > 0) {
-                builder.append(" DEFAULT ").append(defval);
-
-            } else if (field.getType().equals(Byte.TYPE) ||
-                       field.getType().equals(Short.TYPE) ||
-                       field.getType().equals(Integer.TYPE) ||
-                       field.getType().equals(Long.TYPE) ||
-                       field.getType().equals(Float.TYPE) ||
-                       field.getType().equals(Double.TYPE)) {
                 // Java primitive types cannot be null, so we provide a default value for these
                 // columns that matches Java's default for primitive types
-                builder.append(" DEFAULT 0");
+                defaultValue = "0";
 
             } else if (field.getType().equals(Boolean.TYPE)) {
-                builder.append(" DEFAULT ").append(getBooleanDefault());
+                defaultValue = getBooleanDefault();
             }
         }
 
-        return builder.toString();
+        return new ColumnDefinition(type, nullable, unique, defaultValue);
     }
 
     /**

src/java/com/samskivert/jdbc/depot/TableValueGenerator.java

@@ -28,6 +28,7 @@ import java.sql.SQLException;
 import com.samskivert.jdbc.depot.annotation.GeneratedValue;
 import com.samskivert.jdbc.depot.annotation.TableGenerator;
 
+import com.samskivert.jdbc.ColumnDefinition;
 import com.samskivert.jdbc.DatabaseLiaison;
 import com.samskivert.jdbc.JDBCUtil;
 
@@ -45,21 +46,29 @@ public class TableValueGenerator extends ValueGenerator
         _valueColumnName = defStr(tg.valueColumnName(), "value");
     }
 
-    // from interface KeyGenerator
+    @Override // from ValueGenerator
     public boolean isPostFactum ()
     {
         return false;
     }
 
-    // from interface KeyGenerator
+    @Override // from ValueGenerator
     public void init (Connection conn, DatabaseLiaison liaison)
         throws SQLException
     {
+        if (_initialized) {
+            return;
+        }
+        _initialized = true;
+
         // make sure our table exists
         liaison.createTableIfMissing(
             conn, _valueTable,
             new String[] { _pkColumnName, _valueColumnName },
-            new String[] { "VARCHAR(255)", "INTEGER NOT NULL" },
+            new ColumnDefinition[] {
+                new ColumnDefinition("VARCHAR(255)", true, false, null),
+                new ColumnDefinition("INTEGER")
+            },
             null,
             new String[] { _pkColumnName });
 
@@ -76,12 +85,21 @@ public class TableValueGenerator extends ValueGenerator
 
             JDBCUtil.close(stmt);
             stmt = null;
+
+            int initialValue = _initialValue;
+            if (_migrateIfExists) {
+                Integer max = getFieldMaximum(conn, liaison);
+                if (max != null) {
+                    initialValue = 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.setInt(2, initialValue);
             stmt.executeUpdate();
 
         } finally {
@@ -89,7 +107,23 @@ public class TableValueGenerator extends ValueGenerator
         }
     }
 
-    // from interface KeyGenerator
+    @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);
+        }
+    }
+
+    @Override // from ValueGenerator
     public int nextGeneratedValue (Connection conn, DatabaseLiaison liaison)
         throws SQLException
     {
@@ -137,6 +171,7 @@ public class TableValueGenerator extends ValueGenerator
         return value;
     }
 
+    protected boolean _initialized;
     protected String _valueTable;
     protected String _pkColumnName;
     protected String _pkColumnValue;

src/java/com/samskivert/jdbc/depot/ValueGenerator.java

@@ -21,11 +21,17 @@
 package com.samskivert.jdbc.depot;
 
 import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Statement;
 
 import com.samskivert.jdbc.DatabaseLiaison;
+import com.samskivert.jdbc.JDBCUtil;
 import com.samskivert.jdbc.depot.annotation.GeneratedValue;
 
+import static com.samskivert.jdbc.depot.Log.log;
+
 /**
  * Defines the interface to our value generators.
  */
@@ -35,6 +41,7 @@ public abstract class ValueGenerator
     {
         _allocationSize = gv.allocationSize();
         _initialValue = gv.initialValue();
+        _migrateIfExists = gv.migrateIfExists();
         _dm = dm;
         _fm = fm;
     }
@@ -57,6 +64,43 @@ public abstract class ValueGenerator
     public abstract int nextGeneratedValue (Connection conn, DatabaseLiaison liaison)
         throws SQLException;
 
+    /**
+     * Delete all database entities associated with this value generator.
+     */
+    public abstract void delete (Connection conn, DatabaseLiaison liaison)
+        throws SQLException;
+
+    /**
+     * Scans the table associated with this {@link ValueGenerator} and returns either null, if
+     * there are no rows, or an {@link Integer} containing the largest numerical value our
+     * field attains.
+     */
+    protected Integer getFieldMaximum (Connection conn, DatabaseLiaison liaison)
+        throws SQLException
+    {
+        String column = _fm.getColumnName();
+        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));
+            }
+            return null;
+
+        } finally {
+            JDBCUtil.close(stmt);
+        }
+    }
 
     public DepotMarshaller getDepotMarshaller ()
     {
@@ -70,6 +114,8 @@ public abstract class ValueGenerator
 
     protected int _initialValue;
     protected int _allocationSize;
+    protected boolean _migrateIfExists;
+
     protected DepotMarshaller _dm;
     protected FieldMarshaller _fm;
 }

src/java/com/samskivert/jdbc/depot/annotation/GeneratedValue.java

@@ -48,10 +48,21 @@ public @interface GeneratedValue
      */
     int initialValue () default 1;
 
+    /**
+     * If there are rows in our corresponding table, this boolean determines whether or not to
+     * attempt to initialize the generator to the maximum value of our associated field over
+     * those rows. This attribute does not exist in the EJB3 framework.
+     */
+    boolean migrateIfExists () default true;
+
     /**
      * The amount to increment by when allocating id numbers from the generator. The default
      * allocation size is 1. <em>Note:</em> this default differs from the value used by the EJB3
      * persistence framework.
      */
     int allocationSize () default 1;
+
+    /**
+     *
+     */
 }