BugFix: Allow non-primary-key generated fields, e.g. timestamps, coun…

…ters, etc. (#96)

* Allow non-primary-key generated fields, e.g. timestamps, counters, etc.

Currently, HibernateD treats `@Generated` as being inseparable from `@Id`,
it is assumed that any generated field is also the primary key.

However, a database table can have non-primary-key columns that are `@Generated`.
For example, a database that keeps track of a timestamp, which is populated
via a DB trigger or a default value, e.g. `now()`, may exist alongside a
separate primary key.

In order to support this kind of data, the assumption that `@Generated` implies
`@Id` needs to be undone. This PR changes the core logic and also adds a basic
test around generated columns to validate schema generation as well as the ability
to insert and update such records.

* Not sure how that change go in there.

* The transaction test cleanup isn't complete apparently.

* For MySQL, add a UNIQUE constraint for non-PK generated columns.

* Fix syntax error in MySQL when setting UNIQUE.

* Add logic to add defaults for MySQL generated non-integer types.

* Make sure SqlType.BIGINT gets an autoincrement in MySQL.

* Make sure generated parameters can still be manually set.

* Update pgsqldialect to use a default value in schema generation when no generator exists for column type.

* In pgsqldialect, autoincrement becomes the type, not an attribute.

* Remove some debug output.

hdtest/source/embeddedidtest.d

@@ -1,6 +1,5 @@
 module embeddedidtest;
 
-
 import std.algorithm : any;
 
 import hibernated.core;
@@ -159,4 +158,31 @@ class EmbeddedIdTest : HibernateTest {
 
         sess.close();
     }
+
+    @Test("embeddedid.refresh")
+    void refreshTest() {
+        Session sess = sessionFactory.openSession();
+
+        // Create a new record that we can mutate outside the session.
+        Invoice invoice = new Invoice();
+        invoice.invoiceId = new InvoiceId();
+        invoice.invoiceId.vendorNo = "ABC123";
+        invoice.invoiceId.invoiceNo = "L1005-2330";
+        invoice.currency = "EUR";
+        invoice.amountE4 = 54_3200;
+        sess.save(invoice).get!InvoiceId;
+
+        // Modify this entity outside the session using a raw SQL query.
+        sess.doWork(
+                (Connection c) {
+                    Statement stmt = c.createStatement();
+                    scope (exit) stmt.close();
+                    stmt.executeUpdate("UPDATE invoice SET currency = 'USD' "
+                            ~ "WHERE vendor_no = 'ABC123' AND invoice_no = 'L1005-2330'");
+                });
+
+        // Make sure that the entity picks up the out-of-session changes.
+        sess.refresh(invoice);
+        assert(invoice.currency == "USD");
+    }
 }

hdtest/source/generatedtest.d

@@ -0,0 +1,131 @@
+module generatedtest;
+
+import std.datetime;
+
+import hibernated.core;
+
+import testrunner : Test;
+import hibernatetest : HibernateTest;
+
+// A class representing a entity with a single generated key.
+@Entity
+class Generated1 {
+    // Generated, we can leave this off and the DB will create it.
+    @Id @Generated
+    int myId;
+
+    string name;
+}
+
+// A class representing a entity with multiple generated values.
+@Entity
+class Generated2 {
+    // Not generated, this must be set in order to save.
+    @Id
+    int myId;
+
+    // The DB will create this value and it does not need to be set.
+    @Generated
+    int counter1;
+
+    // The DB will create this value and it does not need to be set.
+    @Generated
+    DateTime counter2;
+
+    string name;
+}
+
+class GeneratedTest : HibernateTest {
+    override
+    EntityMetaData buildSchema() {
+        return new SchemaInfoImpl!(Generated1, Generated2);
+    }
+
+    @Test("generated.primary-generated")
+    void creation1Test() {
+        Session sess = sessionFactory.openSession();
+        scope (exit) sess.close();
+
+        Generated1 g1 = new Generated1();
+        g1.name = "Bob";
+        sess.save(g1);
+        // This value should have been detected as empty, populated by the DB, and refreshed.
+        int g1Id = g1.myId;
+        assert(g1Id != 0);
+
+        g1.name = "Barb";
+        sess.update(g1);
+        // The ID should not have changed.
+        assert(g1.myId == g1Id);
+    }
+
+    @Test("generated.mannually-set-id-generated")
+    void manuallySetIdTest() {
+        Session sess = sessionFactory.openSession();
+        scope (exit) sess.close();
+
+        Generated1 g1 = new Generated1();
+        g1.myId = 10;
+        g1.name = "Bob";
+        sess.save(g1);
+        // This value should have been detected as empty, populated by the DB, and refreshed.
+        int g1Id = g1.myId;
+        assert(g1Id == 10);
+
+        g1.name = "Barb";
+        sess.update(g1);
+
+        // Make a new session to avoid caching.
+        sess.close();
+        sess = sessionFactory.openSession();
+        g1 = sess.get!Generated1(10);
+
+        // The ID should not have been generated.
+        assert(g1.myId == g1Id);
+    }
+
+    @Test("generated.non-primary-generated")
+    void creation2Test() {
+        Session sess = sessionFactory.openSession();
+        scope (exit) sess.close();
+
+        Generated2 g2 = new Generated2();
+        g2.myId = 2;
+        g2.name = "Sam";
+        sess.save(g2);
+
+        int g2Id = g2.myId;
+
+        g2.name = "Slom";
+        sess.update(g2);
+
+        // The ID should not have changed.
+        assert(g2Id == g2.myId);
+    }
+
+    @Test("generated.manually-set-non-id-generated")
+    void manuallySetNonIdTest() {
+        Session sess = sessionFactory.openSession();
+        scope (exit) sess.close();
+
+        Generated2 g2 = new Generated2();
+        g2.myId = 3;
+        g2.name = "Sam";
+        g2.counter1 = 11;
+        sess.save(g2);
+
+        int g2Id = g2.myId;
+
+        g2.name = "Slom";
+        sess.update(g2);
+
+        // Make a new session to avoid caching.
+        sess.close();
+        sess = sessionFactory.openSession();
+        g2 = sess.get!Generated2(3);
+
+        // The ID should not have changed.
+        assert(g2Id == g2.myId);
+        assert(g2.counter1 == 11);
+    }
+}

hdtest/source/htestmain.d

@@ -14,9 +14,19 @@ import generaltest : GeneralTest;
 import embeddedtest : EmbeddedTest;
 import embeddedidtest : EmbeddedIdTest;
 import transactiontest : TransactionTest;
+import generatedtest : GeneratedTest;
+
+void enableTraceLogging() {
+    import std.logger : sharedLog, LogLevel, globalLogLevel;
+    (cast() sharedLog).logLevel = LogLevel.trace;
+    globalLogLevel = LogLevel.trace;
+}
 
 int main(string[] args) {
 
+  // Use this to enable trace() logs, useful to inspect generated SQL.
+  enableTraceLogging();
+
   ConnectionParams par;
 
   try {
@@ -38,10 +48,15 @@ int main(string[] args) {
   test3.setConnectionParams(par);
   runTests(test3);
 
-  TransactionTest test4 = new TransactionTest();
+  GeneratedTest test4 = new GeneratedTest();
   test4.setConnectionParams(par);
   runTests(test4);
 
+  // TODO: Some tests that run after this have errors, find out why.
+  TransactionTest test5 = new TransactionTest();
+  test5.setConnectionParams(par);
+  runTests(test5);
+
   writeln("All scenarios worked successfully");
   return 0;
 }

source/hibernated/annotations.d

@@ -168,6 +168,10 @@ struct Table {
 struct Column {
     immutable string name;
     immutable int length;
+    /// Whether the column is included in SQL INSERT statements generated by the persistence provider.
+    immutable bool insertable = true;
+    /// Whether the column is included in SQL UPDATE statements generated by the persistence provider.
+    immutable bool updatable = true;
     //  this(string name) { this.name = name; }
     //  this(string name, int length) { this.name = name; this.length = length; }
     //  this(int length) { this.length = length; }

source/hibernated/dialects/mysqldialect.d

@@ -115,7 +115,15 @@ class MySQLDialect : Dialect {
         bool fk = pi is null;
         string nullablility = !fk && pi.nullable ? " NULL" : " NOT NULL";
         string pk = !fk && pi.key ? " PRIMARY KEY" : "";
-        string autoinc = !fk && pi.generated ? " AUTO_INCREMENT" : "";
+        // MySQL only supports AUTO_INCREMENT for integer types of PRIMARY KEY or UNIQUE.
+        string autoinc = (!fk && pi.generated)
+                ? (sqlType == SqlType.INTEGER || sqlType == SqlType.BIGINT
+                    ? (pi.key
+                         ? " AUTO_INCREMENT"
+                         : " AUTO_INCREMENT UNIQUE")
+                    // Without a generator, use a default so that it it is optional for insert/update.
+                    : " DEFAULT " ~ getImplicitDefaultByType(sqlType))
+                : "";
         string def = "";
         int len = 0;
         string unsigned = "";
@@ -233,4 +241,38 @@ unittest {
 	assert(dialect.quoteSqlString("a\nc") == "'a\\nc'");
 	assert(dialect.quoteIfNeeded("blabla") == "blabla");
 	assert(dialect.quoteIfNeeded("true") == "`true`");
-}
+}
+
+private string getImplicitDefaultByType(SqlType sqlType) {
+    switch (sqlType) {
+        case SqlType.BIGINT:
+        case SqlType.BIT:
+        case SqlType.DECIMAL:
+        case SqlType.DOUBLE:
+        case SqlType.FLOAT:
+        case SqlType.INTEGER:
+        case SqlType.NUMERIC:
+        case SqlType.SMALLINT:
+        case SqlType.TINYINT:
+            return "0";
+        case SqlType.BOOLEAN:
+            return "false";
+        case SqlType.CHAR:
+        case SqlType.LONGNVARCHAR:
+        case SqlType.LONGVARBINARY:
+        case SqlType.LONGVARCHAR:
+        case SqlType.NCHAR:
+        case SqlType.NCLOB:
+        case SqlType.NVARCHAR:
+        case SqlType.VARBINARY:
+        case SqlType.VARCHAR:
+            return "''";
+        case SqlType.DATE:
+        case SqlType.DATETIME:
+            return "'1970-01-01'";
+        case SqlType.TIME:
+            return "'00:00:00'";
+        default:
+            return "''";
+    }
+}

source/hibernated/dialects/pgsqldialect.d

@@ -1,10 +1,10 @@
 /**
- * HibernateD - Object-Relation Mapping for D programming language, with interface similar to Hibernate. 
- * 
+ * HibernateD - Object-Relation Mapping for D programming language, with interface similar to Hibernate.
+ *
  * Source file hibernated/dialects/sqlitedialect.d.
  *
  * This module contains implementation of PGSQLDialect class which provides implementation specific SQL syntax information.
- * 
+ *
  * Copyright: Copyright 2013
  * License:   $(LINK www.boost.org/LICENSE_1_0.txt, Boost License 1.0).
  * Author:   Vadim Lopatin
@@ -19,7 +19,7 @@ import hibernated.type;
 import ddbc.core : SqlType;
 
 
-string[] PGSQL_RESERVED_WORDS = 
+string[] PGSQL_RESERVED_WORDS =
     [
      "ABORT",
      "ACTION",
@@ -151,20 +151,24 @@ class PGSQLDialect : Dialect {
     override char closeQuote() const { return '"'; }
     ///The character specific to this dialect used to begin a quoted identifier.
     override char  openQuote() const { return '"'; }
-    
+
     // returns string like "BIGINT(20) NOT NULL" or "VARCHAR(255) NULL"
     override string getColumnTypeDefinition(const PropertyInfo pi, const PropertyInfo overrideTypeFrom = null) {
         immutable Type type = overrideTypeFrom !is null ? overrideTypeFrom.columnType : pi.columnType;
         immutable SqlType sqlType = type.getSqlType();
         bool fk = pi is null;
         string nullablility = !fk && pi.nullable ? " NULL" : " NOT NULL";
         string pk = !fk && pi.key ? " PRIMARY KEY" : "";
+        string autoinc = "";
         if (!fk && pi.generated) {
-            if (sqlType == SqlType.SMALLINT || sqlType == SqlType.TINYINT)
-                return "SERIAL PRIMARY KEY";
-            if (sqlType == SqlType.INTEGER)
-                return "SERIAL PRIMARY KEY";
-            return "BIGSERIAL PRIMARY KEY";
+            if (sqlType == SqlType.SMALLINT || sqlType == SqlType.TINYINT || sqlType == SqlType.INTEGER) {
+                return "SERIAL" ~ pk;
+            } else if (sqlType == SqlType.BIGINT) {
+                return "BIGSERIAL" ~ pk;
+            } else {
+                // Without a generator, use a default so that it is optional for insert/update.
+                autoinc = " DEFAULT " ~ getImplicitDefaultByType(sqlType);
+            }
         }
         string def = "";
         int len = 0;
@@ -174,7 +178,7 @@ class PGSQLDialect : Dialect {
         if (cast(StringType)type !is null) {
             len = (cast(StringType)type).length;
         }
-        string modifiers = nullablility ~ def ~ pk;
+        string modifiers = nullablility ~ def ~ pk ~ autoinc;
         string lenmodifiers = "(" ~ to!string(len > 0 ? len : 255) ~ ")" ~ modifiers;
         switch (sqlType) {
             case SqlType.BIGINT:
@@ -219,15 +223,15 @@ class PGSQLDialect : Dialect {
                 return "TEXT";
         }
     }
-    
+
     override string getCheckTableExistsSQL(string tableName) {
         return "select relname from pg_class where relname = " ~ quoteSqlString(tableName) ~ " and relkind='r'";
     }
-    
+
     override string getUniqueIndexItemSQL(string indexName, string[] columnNames) {
         return "UNIQUE " ~ createFieldListSQL(columnNames);
     }
-    
+
     /// for some of RDBMS it's necessary to pass additional clauses in query to get generated value (e.g. in Postgres - " returing id"
     override string appendInsertToFetchGeneratedKey(string query, const EntityInfo entity) {
         return query ~ " RETURNING " ~ quoteIfNeeded(entity.getKeyProperty().columnName);
@@ -238,3 +242,36 @@ class PGSQLDialect : Dialect {
     }
 }
 
+private string getImplicitDefaultByType(SqlType sqlType) {
+    switch (sqlType) {
+        case SqlType.BIGINT:
+        case SqlType.BIT:
+        case SqlType.DECIMAL:
+        case SqlType.DOUBLE:
+        case SqlType.FLOAT:
+        case SqlType.INTEGER:
+        case SqlType.NUMERIC:
+        case SqlType.SMALLINT:
+        case SqlType.TINYINT:
+            return "0";
+        case SqlType.BOOLEAN:
+            return "false";
+        case SqlType.CHAR:
+        case SqlType.LONGNVARCHAR:
+        case SqlType.LONGVARBINARY:
+        case SqlType.LONGVARCHAR:
+        case SqlType.NCHAR:
+        case SqlType.NCLOB:
+        case SqlType.NVARCHAR:
+        case SqlType.VARBINARY:
+        case SqlType.VARCHAR:
+            return "''";
+        case SqlType.DATE:
+        case SqlType.DATETIME:
+            return "'1970-01-01'";
+        case SqlType.TIME:
+            return "'00:00:00'";
+        default:
+            return "''";
+    }
+}