Skip to content

Commit c219b4d

Browse files
vnayarvnayar
authored
BugFix: Allow non-primary-key generated fields, e.g. timestamps, counters, 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.
1 parent 223a71f commit c219b4d

File tree

9 files changed

+486
-95
lines changed

9 files changed

+486
-95
lines changed

‎hdtest/source/embeddedidtest.d

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
module embeddedidtest;
22

3-
43
import std.algorithm : any;
54

65
import hibernated.core;
@@ -159,4 +158,31 @@ class EmbeddedIdTest : HibernateTest {
159158

160159
sess.close();
161160
}
161+
162+
@Test("embeddedid.refresh")
163+
void refreshTest() {
164+
Session sess = sessionFactory.openSession();
165+
166+
// Create a new record that we can mutate outside the session.
167+
Invoice invoice = new Invoice();
168+
invoice.invoiceId = new InvoiceId();
169+
invoice.invoiceId.vendorNo = "ABC123";
170+
invoice.invoiceId.invoiceNo = "L1005-2330";
171+
invoice.currency = "EUR";
172+
invoice.amountE4 = 54_3200;
173+
sess.save(invoice).get!InvoiceId;
174+
175+
// Modify this entity outside the session using a raw SQL query.
176+
sess.doWork(
177+
(Connection c) {
178+
Statement stmt = c.createStatement();
179+
scope (exit) stmt.close();
180+
stmt.executeUpdate("UPDATE invoice SET currency = 'USD' "
181+
~ "WHERE vendor_no = 'ABC123' AND invoice_no = 'L1005-2330'");
182+
});
183+
184+
// Make sure that the entity picks up the out-of-session changes.
185+
sess.refresh(invoice);
186+
assert(invoice.currency == "USD");
187+
}
162188
}

‎hdtest/source/generatedtest.d

+131
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
module generatedtest;
2+
3+
import std.datetime;
4+
5+
import hibernated.core;
6+
7+
import testrunner : Test;
8+
import hibernatetest : HibernateTest;
9+
10+
// A class representing a entity with a single generated key.
11+
@Entity
12+
class Generated1 {
13+
// Generated, we can leave this off and the DB will create it.
14+
@Id @Generated
15+
int myId;
16+
17+
string name;
18+
}
19+
20+
// A class representing a entity with multiple generated values.
21+
@Entity
22+
class Generated2 {
23+
// Not generated, this must be set in order to save.
24+
@Id
25+
int myId;
26+
27+
// The DB will create this value and it does not need to be set.
28+
@Generated
29+
int counter1;
30+
31+
// The DB will create this value and it does not need to be set.
32+
@Generated
33+
DateTime counter2;
34+
35+
string name;
36+
}
37+
38+
class GeneratedTest : HibernateTest {
39+
override
40+
EntityMetaData buildSchema() {
41+
return new SchemaInfoImpl!(Generated1, Generated2);
42+
}
43+
44+
@Test("generated.primary-generated")
45+
void creation1Test() {
46+
Session sess = sessionFactory.openSession();
47+
scope (exit) sess.close();
48+
49+
Generated1 g1 = new Generated1();
50+
g1.name = "Bob";
51+
sess.save(g1);
52+
// This value should have been detected as empty, populated by the DB, and refreshed.
53+
int g1Id = g1.myId;
54+
assert(g1Id != 0);
55+
56+
g1.name = "Barb";
57+
sess.update(g1);
58+
// The ID should not have changed.
59+
assert(g1.myId == g1Id);
60+
}
61+
62+
@Test("generated.mannually-set-id-generated")
63+
void manuallySetIdTest() {
64+
Session sess = sessionFactory.openSession();
65+
scope (exit) sess.close();
66+
67+
Generated1 g1 = new Generated1();
68+
g1.myId = 10;
69+
g1.name = "Bob";
70+
sess.save(g1);
71+
// This value should have been detected as empty, populated by the DB, and refreshed.
72+
int g1Id = g1.myId;
73+
assert(g1Id == 10);
74+
75+
g1.name = "Barb";
76+
sess.update(g1);
77+
78+
// Make a new session to avoid caching.
79+
sess.close();
80+
sess = sessionFactory.openSession();
81+
g1 = sess.get!Generated1(10);
82+
83+
// The ID should not have been generated.
84+
assert(g1.myId == g1Id);
85+
}
86+
87+
@Test("generated.non-primary-generated")
88+
void creation2Test() {
89+
Session sess = sessionFactory.openSession();
90+
scope (exit) sess.close();
91+
92+
Generated2 g2 = new Generated2();
93+
g2.myId = 2;
94+
g2.name = "Sam";
95+
sess.save(g2);
96+
97+
int g2Id = g2.myId;
98+
99+
g2.name = "Slom";
100+
sess.update(g2);
101+
102+
// The ID should not have changed.
103+
assert(g2Id == g2.myId);
104+
}
105+
106+
@Test("generated.manually-set-non-id-generated")
107+
void manuallySetNonIdTest() {
108+
Session sess = sessionFactory.openSession();
109+
scope (exit) sess.close();
110+
111+
Generated2 g2 = new Generated2();
112+
g2.myId = 3;
113+
g2.name = "Sam";
114+
g2.counter1 = 11;
115+
sess.save(g2);
116+
117+
int g2Id = g2.myId;
118+
119+
g2.name = "Slom";
120+
sess.update(g2);
121+
122+
// Make a new session to avoid caching.
123+
sess.close();
124+
sess = sessionFactory.openSession();
125+
g2 = sess.get!Generated2(3);
126+
127+
// The ID should not have changed.
128+
assert(g2Id == g2.myId);
129+
assert(g2.counter1 == 11);
130+
}
131+
}

‎hdtest/source/htestmain.d

+16-1
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,19 @@ import generaltest : GeneralTest;
1414
import embeddedtest : EmbeddedTest;
1515
import embeddedidtest : EmbeddedIdTest;
1616
import transactiontest : TransactionTest;
17+
import generatedtest : GeneratedTest;
18+
19+
void enableTraceLogging() {
20+
import std.logger : sharedLog, LogLevel, globalLogLevel;
21+
(cast() sharedLog).logLevel = LogLevel.trace;
22+
globalLogLevel = LogLevel.trace;
23+
}
1724

1825
int main(string[] args) {
1926

27+
// Use this to enable trace() logs, useful to inspect generated SQL.
28+
enableTraceLogging();
29+
2030
ConnectionParams par;
2131

2232
try {
@@ -38,10 +48,15 @@ int main(string[] args) {
3848
test3.setConnectionParams(par);
3949
runTests(test3);
4050

41-
TransactionTest test4 = new TransactionTest();
51+
GeneratedTest test4 = new GeneratedTest();
4252
test4.setConnectionParams(par);
4353
runTests(test4);
4454

55+
// TODO: Some tests that run after this have errors, find out why.
56+
TransactionTest test5 = new TransactionTest();
57+
test5.setConnectionParams(par);
58+
runTests(test5);
59+
4560
writeln("All scenarios worked successfully");
4661
return 0;
4762
}

‎source/hibernated/annotations.d

+4
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ struct Table {
168168
struct Column {
169169
immutable string name;
170170
immutable int length;
171+
/// Whether the column is included in SQL INSERT statements generated by the persistence provider.
172+
immutable bool insertable = true;
173+
/// Whether the column is included in SQL UPDATE statements generated by the persistence provider.
174+
immutable bool updatable = true;
171175
// this(string name) { this.name = name; }
172176
// this(string name, int length) { this.name = name; this.length = length; }
173177
// this(int length) { this.length = length; }

‎source/hibernated/dialects/mysqldialect.d

+44-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,15 @@ class MySQLDialect : Dialect {
115115
bool fk = pi is null;
116116
string nullablility = !fk && pi.nullable ? " NULL" : " NOT NULL";
117117
string pk = !fk && pi.key ? " PRIMARY KEY" : "";
118-
string autoinc = !fk && pi.generated ? " AUTO_INCREMENT" : "";
118+
// MySQL only supports AUTO_INCREMENT for integer types of PRIMARY KEY or UNIQUE.
119+
string autoinc = (!fk && pi.generated)
120+
? (sqlType == SqlType.INTEGER || sqlType == SqlType.BIGINT
121+
? (pi.key
122+
? " AUTO_INCREMENT"
123+
: " AUTO_INCREMENT UNIQUE")
124+
// Without a generator, use a default so that it it is optional for insert/update.
125+
: " DEFAULT " ~ getImplicitDefaultByType(sqlType))
126+
: "";
119127
string def = "";
120128
int len = 0;
121129
string unsigned = "";
@@ -233,4 +241,38 @@ unittest {
233241
assert(dialect.quoteSqlString("a\nc") == "'a\\nc'");
234242
assert(dialect.quoteIfNeeded("blabla") == "blabla");
235243
assert(dialect.quoteIfNeeded("true") == "`true`");
236-
}
244+
}
245+
246+
private string getImplicitDefaultByType(SqlType sqlType) {
247+
switch (sqlType) {
248+
case SqlType.BIGINT:
249+
case SqlType.BIT:
250+
case SqlType.DECIMAL:
251+
case SqlType.DOUBLE:
252+
case SqlType.FLOAT:
253+
case SqlType.INTEGER:
254+
case SqlType.NUMERIC:
255+
case SqlType.SMALLINT:
256+
case SqlType.TINYINT:
257+
return "0";
258+
case SqlType.BOOLEAN:
259+
return "false";
260+
case SqlType.CHAR:
261+
case SqlType.LONGNVARCHAR:
262+
case SqlType.LONGVARBINARY:
263+
case SqlType.LONGVARCHAR:
264+
case SqlType.NCHAR:
265+
case SqlType.NCLOB:
266+
case SqlType.NVARCHAR:
267+
case SqlType.VARBINARY:
268+
case SqlType.VARCHAR:
269+
return "''";
270+
case SqlType.DATE:
271+
case SqlType.DATETIME:
272+
return "'1970-01-01'";
273+
case SqlType.TIME:
274+
return "'00:00:00'";
275+
default:
276+
return "''";
277+
}
278+
}

‎source/hibernated/dialects/pgsqldialect.d

+51-14
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
2-
* HibernateD - Object-Relation Mapping for D programming language, with interface similar to Hibernate.
3-
*
2+
* HibernateD - Object-Relation Mapping for D programming language, with interface similar to Hibernate.
3+
*
44
* Source file hibernated/dialects/sqlitedialect.d.
55
*
66
* This module contains implementation of PGSQLDialect class which provides implementation specific SQL syntax information.
7-
*
7+
*
88
* Copyright: Copyright 2013
99
* License: $(LINK www.boost.org/LICENSE_1_0.txt, Boost License 1.0).
1010
* Author: Vadim Lopatin
@@ -19,7 +19,7 @@ import hibernated.type;
1919
import ddbc.core : SqlType;
2020

2121

22-
string[] PGSQL_RESERVED_WORDS =
22+
string[] PGSQL_RESERVED_WORDS =
2323
[
2424
"ABORT",
2525
"ACTION",
@@ -151,20 +151,24 @@ class PGSQLDialect : Dialect {
151151
override char closeQuote() const { return '"'; }
152152
///The character specific to this dialect used to begin a quoted identifier.
153153
override char openQuote() const { return '"'; }
154-
154+
155155
// returns string like "BIGINT(20) NOT NULL" or "VARCHAR(255) NULL"
156156
override string getColumnTypeDefinition(const PropertyInfo pi, const PropertyInfo overrideTypeFrom = null) {
157157
immutable Type type = overrideTypeFrom !is null ? overrideTypeFrom.columnType : pi.columnType;
158158
immutable SqlType sqlType = type.getSqlType();
159159
bool fk = pi is null;
160160
string nullablility = !fk && pi.nullable ? " NULL" : " NOT NULL";
161161
string pk = !fk && pi.key ? " PRIMARY KEY" : "";
162+
string autoinc = "";
162163
if (!fk && pi.generated) {
163-
if (sqlType == SqlType.SMALLINT || sqlType == SqlType.TINYINT)
164-
return "SERIAL PRIMARY KEY";
165-
if (sqlType == SqlType.INTEGER)
166-
return "SERIAL PRIMARY KEY";
167-
return "BIGSERIAL PRIMARY KEY";
164+
if (sqlType == SqlType.SMALLINT || sqlType == SqlType.TINYINT || sqlType == SqlType.INTEGER) {
165+
return "SERIAL" ~ pk;
166+
} else if (sqlType == SqlType.BIGINT) {
167+
return "BIGSERIAL" ~ pk;
168+
} else {
169+
// Without a generator, use a default so that it is optional for insert/update.
170+
autoinc = " DEFAULT " ~ getImplicitDefaultByType(sqlType);
171+
}
168172
}
169173
string def = "";
170174
int len = 0;
@@ -174,7 +178,7 @@ class PGSQLDialect : Dialect {
174178
if (cast(StringType)type !is null) {
175179
len = (cast(StringType)type).length;
176180
}
177-
string modifiers = nullablility ~ def ~ pk;
181+
string modifiers = nullablility ~ def ~ pk ~ autoinc;
178182
string lenmodifiers = "(" ~ to!string(len > 0 ? len : 255) ~ ")" ~ modifiers;
179183
switch (sqlType) {
180184
case SqlType.BIGINT:
@@ -219,15 +223,15 @@ class PGSQLDialect : Dialect {
219223
return "TEXT";
220224
}
221225
}
222-
226+
223227
override string getCheckTableExistsSQL(string tableName) {
224228
return "select relname from pg_class where relname = " ~ quoteSqlString(tableName) ~ " and relkind='r'";
225229
}
226-
230+
227231
override string getUniqueIndexItemSQL(string indexName, string[] columnNames) {
228232
return "UNIQUE " ~ createFieldListSQL(columnNames);
229233
}
230-
234+
231235
/// for some of RDBMS it's necessary to pass additional clauses in query to get generated value (e.g. in Postgres - " returing id"
232236
override string appendInsertToFetchGeneratedKey(string query, const EntityInfo entity) {
233237
return query ~ " RETURNING " ~ quoteIfNeeded(entity.getKeyProperty().columnName);
@@ -238,3 +242,36 @@ class PGSQLDialect : Dialect {
238242
}
239243
}
240244

245+
private string getImplicitDefaultByType(SqlType sqlType) {
246+
switch (sqlType) {
247+
case SqlType.BIGINT:
248+
case SqlType.BIT:
249+
case SqlType.DECIMAL:
250+
case SqlType.DOUBLE:
251+
case SqlType.FLOAT:
252+
case SqlType.INTEGER:
253+
case SqlType.NUMERIC:
254+
case SqlType.SMALLINT:
255+
case SqlType.TINYINT:
256+
return "0";
257+
case SqlType.BOOLEAN:
258+
return "false";
259+
case SqlType.CHAR:
260+
case SqlType.LONGNVARCHAR:
261+
case SqlType.LONGVARBINARY:
262+
case SqlType.LONGVARCHAR:
263+
case SqlType.NCHAR:
264+
case SqlType.NCLOB:
265+
case SqlType.NVARCHAR:
266+
case SqlType.VARBINARY:
267+
case SqlType.VARCHAR:
268+
return "''";
269+
case SqlType.DATE:
270+
case SqlType.DATETIME:
271+
return "'1970-01-01'";
272+
case SqlType.TIME:
273+
return "'00:00:00'";
274+
default:
275+
return "''";
276+
}
277+
}

0 commit comments

Comments
 (0)