diff --git a/src/java/com/samskivert/jdbc/DatabaseLiaison.java b/src/java/com/samskivert/jdbc/DatabaseLiaison.java index f5bd835e..03ead501 100644 --- a/src/java/com/samskivert/jdbc/DatabaseLiaison.java +++ b/src/java/com/samskivert/jdbc/DatabaseLiaison.java @@ -58,18 +58,26 @@ public interface DatabaseLiaison public boolean isTransientException (SQLException sqe); /** - * This method deletes the column value auto-generator described in {@link - * #lastInsertedId}. + * Attempts as dialect-agnostic an interface as possible to the ability of certain databases to + * auto-generated numerical values for i.e. key columns; there is MySQL's AUTO_INCREMENT and + * PostgreSQL's DEFAULT nextval(sequence), for example. */ - public void deleteGenerator (Connection conn, String tableName, String columnName) + public int lastInsertedId (Connection conn, String table, String column) throws SQLException; /** - * This method attempts as dialect-agnostic an interface as possible to the ability of certain - * databases to auto-generated numerical values for i.e. key columns; there is MySQL's - * AUTO_INCREMENT and PostgreSQL's DEFAULT nextval(sequence), for example. + * Initializes the column value auto-generator described in {@link #lastInsertedId}. This + * should be idempotent (meaning the generator may already exist in which case this method + * should have no negative effect like resetting it). */ - public int lastInsertedId (Connection conn, String table, String column) + public void createGenerator (Connection conn, String tableName, String columnName, + int initialValue) + throws SQLException; + + /** + * Deletes the column value auto-generator described in {@link #lastInsertedId}. + */ + public void deleteGenerator (Connection conn, String tableName, String columnName) throws SQLException; /** diff --git a/src/java/com/samskivert/jdbc/DefaultLiaison.java b/src/java/com/samskivert/jdbc/DefaultLiaison.java index 12ac2e84..381ef0ac 100644 --- a/src/java/com/samskivert/jdbc/DefaultLiaison.java +++ b/src/java/com/samskivert/jdbc/DefaultLiaison.java @@ -47,6 +47,13 @@ public class DefaultLiaison extends BaseLiaison return false; } + // from DatabaseLiaison + public void createGenerator (Connection conn, String tableName, String columnName, int initValue) + throws SQLException + { + // nothing doing + } + // from DatabaseLiaison public void deleteGenerator (Connection conn, String tableName, String columnName) throws SQLException diff --git a/src/java/com/samskivert/jdbc/MySQLLiaison.java b/src/java/com/samskivert/jdbc/MySQLLiaison.java index 15e0c0d2..dd43976d 100644 --- a/src/java/com/samskivert/jdbc/MySQLLiaison.java +++ b/src/java/com/samskivert/jdbc/MySQLLiaison.java @@ -51,6 +51,14 @@ public class MySQLLiaison extends BaseLiaison msg.indexOf("Broken pipe") != -1)); } + // from DatabaseLiaison + public void createGenerator (Connection conn, String tableName, String columnName, int initValue) + throws SQLException + { + // TODO: is there any way we can set the initial AUTO_INCREMENT value? + } + + // from DatabaseLiaison public void deleteGenerator (Connection conn, String tableName, String columnName) throws SQLException { diff --git a/src/java/com/samskivert/jdbc/PostgreSQLLiaison.java b/src/java/com/samskivert/jdbc/PostgreSQLLiaison.java index acacd1a2..344cd749 100644 --- a/src/java/com/samskivert/jdbc/PostgreSQLLiaison.java +++ b/src/java/com/samskivert/jdbc/PostgreSQLLiaison.java @@ -58,23 +58,34 @@ public class PostgreSQLLiaison extends BaseLiaison // sequences and DEFAULT nextval(sequence) modifiers in the ID columns. To get the next ID, // we use the currval() method which is set in a database sessions when any given sequence // is incremented. - Statement stmt = null; - + Statement stmt = conn.createStatement(); try { - stmt = conn.createStatement(); ResultSet rs = stmt.executeQuery( "select currval('\"" + table + "_" + column + "_seq\"')"); - if (rs.next()) { - return rs.getInt(1); - } else { - return -1; - } - + return rs.next() ? rs.getInt(1) :-1; } finally { JDBCUtil.close(stmt); } } + // from DatabaseLiaison + public void createGenerator (Connection conn, String tableName, String columnName, int initValue) + throws SQLException + { + if (initValue == 1) { + return; // that's the default! yay, do nothing + } + + String seqname = "\"" + tableName + "_" + columnName + "_seq\""; + Statement stmt = conn.createStatement(); + try { + stmt.executeQuery("select setval('" + seqname + "', " + initValue + ", false)"); + } finally { + JDBCUtil.close(stmt); + } + log.info("Initial value of " + seqname + " set to " + initValue + "."); + } + // from DatabaseLiaison public void deleteGenerator (Connection conn, String table, String column) throws SQLException diff --git a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java index 0697d217..c1ec6d3c 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java +++ b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java @@ -705,9 +705,9 @@ public class DepotMarshaller getTableName() + "_" + idx.name(), idx.unique()); } - // initialize our value generators + // create our value generators for (ValueGenerator vg : _valueGenerators.values()) { - vg.init(conn, liaison); + vg.create(conn, liaison); } // and its full text search indexes @@ -744,6 +744,13 @@ public class DepotMarshaller // this is a little silly, but we need a copy for name disambiguation later Set indicesCopy = new HashSet(metaData.indexColumns.keySet()); + // figure out which columns we have in the table now, so that when all is said and done we + // can see what new columns we have in the table and run the creation code for any value + // generators that are defined on those columns (we can't just track the columns we add in + // our automatic migrations because someone might register custom migrations that add + // columns specially) + Set preMigrateColumns = new HashSet(metaData.tableColumns); + // add any missing columns for (String fname : _columnFields) { final FieldMarshaller fmarsh = _fields.get(fname); @@ -896,14 +903,31 @@ public class DepotMarshaller } } - // last of all (re-)initialize our value generators, since one might've been added - if (_valueGenerators.size() > 0) { + // now reload our table metadata so that we can see what columns we have now + metaData = TableMetaData.load(ctx, getTableName()); + + // initialize value generators for any columns that have been newly added + for (String column : metaData.tableColumns) { + if (preMigrateColumns.contains(column)) { + continue; + } + + // see if we have a value generator for this new column + final ValueGenerator valgen = _valueGenerators.get(column); + if (valgen == null) { + continue; + } + + // note: if someone renames a column that has an identity value generator, things will + // break because Postgres automatically creates a table_column_seq sequence that is + // used to generate values for that column and god knows what happens when that is + // renamed; plus we're potentially going to try to reinitialize it if it has a non-zero + // 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 int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { - for (ValueGenerator vg : _valueGenerators.values()) { - vg.init(conn, liaison); - } + valgen.create(conn, liaison); return 0; } }); diff --git a/src/java/com/samskivert/jdbc/depot/IdentityValueGenerator.java b/src/java/com/samskivert/jdbc/depot/IdentityValueGenerator.java index 98706615..84d7f458 100644 --- a/src/java/com/samskivert/jdbc/depot/IdentityValueGenerator.java +++ b/src/java/com/samskivert/jdbc/depot/IdentityValueGenerator.java @@ -43,10 +43,10 @@ public class IdentityValueGenerator extends ValueGenerator } @Override // from ValueGenerator - public void init (Connection conn, DatabaseLiaison liaison) + public void create (Connection conn, DatabaseLiaison liaison) throws SQLException { - // identity value generators are auto-created by the database + liaison.createGenerator(conn, _dm.getTableName(), _fm.getColumnName(), _initialValue); } @Override // from ValueGenerator diff --git a/src/java/com/samskivert/jdbc/depot/TableValueGenerator.java b/src/java/com/samskivert/jdbc/depot/TableValueGenerator.java index c0606300..602c71ac 100644 --- a/src/java/com/samskivert/jdbc/depot/TableValueGenerator.java +++ b/src/java/com/samskivert/jdbc/depot/TableValueGenerator.java @@ -54,14 +54,9 @@ public class TableValueGenerator extends ValueGenerator } @Override // from ValueGenerator - public void init (Connection conn, DatabaseLiaison liaison) + public void create (Connection conn, DatabaseLiaison liaison) throws SQLException { - if (_initialized) { - return; - } - _initialized = true; - // make sure our table exists liaison.createTableIfMissing( conn, _valueTable, @@ -172,7 +167,6 @@ public class TableValueGenerator extends ValueGenerator return value; } - protected boolean _initialized; protected String _valueTable; protected String _pkColumnName; protected String _pkColumnValue; diff --git a/src/java/com/samskivert/jdbc/depot/ValueGenerator.java b/src/java/com/samskivert/jdbc/depot/ValueGenerator.java index c8bca36d..e2e75f0e 100644 --- a/src/java/com/samskivert/jdbc/depot/ValueGenerator.java +++ b/src/java/com/samskivert/jdbc/depot/ValueGenerator.java @@ -52,9 +52,13 @@ public abstract class ValueGenerator public abstract boolean isPostFactum (); /** - * Ensures the generator is prepared for operation, creating it (only) if necessary. + * Ensures the generator is prepared for operation, creating it if necessary. Care is taken to + * only run this the first time a column is created. However, if a column with a value + * generator is renamed, we can't distinguish that from a newly created column and we call this + * method again on the renamed column. If it's possible to not fail in that circumstance, try + * to avoid doing so. */ - public abstract void init (Connection conn, DatabaseLiaison liaison) + public abstract void create (Connection conn, DatabaseLiaison liaison) throws SQLException; /**