Amazingly, we had apparently never tried to insert a preconfigured row into a

table that had field generators on its primary key. The code just blindly
overwrite the supplied primary key and ran the generators anyway.

We now only run the generators if the record has no primary key. This means we
can't use field generators on non-primary-key fields, but I'm not sure we ever
meant to support that in the first place.

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

@@ -358,7 +358,9 @@ public class DepotMarshaller<T extends PersistentRecord>
             int nulls = 0;
             for (int ii = 0; ii < _pkColumns.size(); ii++) {
                 FieldMarshaller<?> field = _pkColumns.get(ii);
-                if ((values[ii] = (Comparable<?>)field.getField().get(object)) == null) {
+                values[ii] = (Comparable<?>)field.getField().get(object);
+                if (values[ii] == null ||
+                    (values[ii] instanceof Number && ((Number)values[ii]).intValue() == 0)) {
                     nulls++;
                 }
             }
@@ -474,14 +476,14 @@ public class DepotMarshaller<T extends PersistentRecord>
     }
 
     /**
-     * Go through the registered {@link ValueGenerator}s for our persistent object and run the
+     * Go through the registered {@link ValueGenerator}s for our persistent object and run the ones
-     * ones that match the current postFactum phase, filling in the fields on the supplied object
+     * that match the current postFactum phase, filling in the fields on the supplied object while
-     * while we go. This method used to generate a key; that is now a separate step.
+     * we go.
      *
-     * The return value is only non-empty for the !postFactum phase, in which case it is a set
+     * The return value is only non-empty for the !postFactum phase, in which case it is a set of
-     * of field names that are associated with {@link IdentityValueGenerator}, because these need
+     * field names that are associated with {@link IdentityValueGenerator}, because these need
-     * special handling in the INSERT (specifically, 'DEFAULT' must be supplied as a value in
+     * special handling in the INSERT (specifically, 'DEFAULT' must be supplied as a value in the
-     * the eventual SQL).
+     * eventual SQL).
      */
     public Set<String> generateFieldValues (
         Connection conn, DatabaseLiaison liaison, Object po, boolean postFactum)

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

@@ -315,12 +315,11 @@ public abstract class DepotRepository
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
             public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
-                // set any auto-generated column values
-                Set<String> identityFields =
-                    marsh.generateFieldValues(conn, liaison, _result, false);
-
                 // if needed, update our modifier's key so that it can cache our results
+                Set<String> identityFields = Collections.emptySet();
                 if (_key == null) {
+                    // set any auto-generated column values
+                    identityFields = marsh.generateFieldValues(conn, liaison, _result, false);
                     updateKey(marsh.getPrimaryKey(_result, false));
                 }
 
@@ -329,15 +328,13 @@ public abstract class DepotRepository
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     int mods = stmt.executeUpdate();
-
+                    // run any post-factum value generators and potentially generate our key
-                    // run any post-factum value generators
-                    marsh.generateFieldValues(conn, liaison, _result, true);
-
-                    // and check once more if a key now exists
                     if (_key == null) {
+                        marsh.generateFieldValues(conn, liaison, _result, true);
                         updateKey(marsh.getPrimaryKey(_result, false));
                     }
                     return mods;
+
                 } finally {
                     JDBCUtil.close(stmt);
                 }
@@ -739,13 +736,12 @@ public abstract class DepotRepository
                         JDBCUtil.close(stmt);
                     }
 
-                    // if the update modified zero rows or the primary key was obviously unset, do
+                    // if the update modified zero rows or the primary key was unset, insert
-                    // an insertion: first, set any auto-generated column values
+                    Set<String> identityFields = Collections.emptySet();
-                    Set<String> identityFields =
-                        marsh.generateFieldValues(conn, liaison, _result, false);
-
-                    // update our modifier's key so that it can cache our results
                     if (_key == null) {
+                        // first, set any auto-generated column values
+                        identityFields = marsh.generateFieldValues(conn, liaison, _result, false);
+                        // update our modifier's key so that it can cache our results
                         updateKey(marsh.getPrimaryKey(_result, false));
                     }
 
@@ -754,11 +750,9 @@ public abstract class DepotRepository
                     stmt = builder.prepare(conn);
                     int mods = stmt.executeUpdate();
 
-                    // run any post-factum value generators
+                    // run any post-factum value generators and potentially generate our key
-                    marsh.generateFieldValues(conn, liaison, _result, true);
-
-                    // and check once more if a key now exists
                     if (_key == null) {
+                        marsh.generateFieldValues(conn, liaison, _result, true);
                         updateKey(marsh.getPrimaryKey(_result, false));
                     }
                     created[0] = true;