diff --git a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java index 0697d21..c1ec6d3 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 9870661..84d7f45 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 c060630..602c71a 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 c8bca36..e2e75f0 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; /**