Refactored DDL generation to correctly handle quoted identifiers #2603
Conversation
rPraml
force-pushed
the
remove-lowertablename
branch
from
cf45fab to
7e3fbbb
Compare
rPraml
changed the title
| /** | ||
| * Trim off the platform quoted identifier quotes like [ ' and ". | ||
| */ | ||
| public boolean notQuoted(String tableName) { | ||
| public String trimQuotes(String identifier) { |
| if (dbName.charAt(0) == BACK_TICK) { | ||
| if (dbName.charAt(dbName.length() - 1) == BACK_TICK) { | ||
| if (isQuote(dbName.charAt(0))) { | ||
| if (isQuote(dbName.charAt(dbName.length() - 1))) { |
There was a problem hiding this comment.
this method converts any quoted identifier into the platform specific form
| if (column != null) { | ||
| target.append(' ').append(column); | ||
| target.append(' ').append(platformDdl.quote(column)); |
There was a problem hiding this comment.
we need to quote table & column name here
| @@ -755,7 +741,7 @@ private void alterColumnComment(DdlWrite writer, AlterColumn alterColumn) { | |||
| * Return the name of the history table given the base table name. | |||
| */ | |||
| protected String historyTable(String baseTable) { | |||
| return baseTable + historyTableSuffix; | |||
| return naming.normaliseTable(baseTable) + historyTableSuffix; | |||
There was a problem hiding this comment.
baseTable must always be normalized (=stripped from identifiers)
Note to myself: I think, we must pay attention for an optional scheme/catalog here (see SqlServerDdl)
|
|
||
| .append(" prepare stmt from 'alter table ").append(lowerTableName(tableName)) | ||
| .append("' and tabname = '").append(naming.normaliseTable(tableName).toUpperCase()).append("') then\n") |
There was a problem hiding this comment.
Note to myself: check if this match should be changed to and lcase(tabname) =
| if (addColumn == null) { | ||
| addColumn = new AddColumn(); | ||
| addColumn.setTableName(name); | ||
| if (withHistory) { | ||
| if (newTable.isWithHistory()) { |
There was a problem hiding this comment.
We must enable history only, when the new table has history.
if you disable history, then the old table has history, but not the new one
| create table PERSONS ( | ||
| ID bigint generated by default as identity (start with 1000 increment by 40) not null, | ||
| SURNAME varchar(64) not null, | ||
| NAME varchar(64) not null, |
There was a problem hiding this comment.
@rbygrave due the removal of "lowerTableName", we will get uppercase names here.
I think, this is OK, because it is specified as @Table(name = "PERSONS") / @Column(name = "ID")
If you want lower case names, specify them in the annotation
| @Table(name = "`table`") | ||
| @Entity | ||
| @History | ||
| public class ETable { |
There was a problem hiding this comment.
This is the "Reserved-keyword" entity. It uses a keyword for each fied and tries to address as many as possible features during DDL generation
|
|
||
| @Table(name = "`table`") | ||
| @Entity | ||
| @History // FIXME: remove later |
There was a problem hiding this comment.
@rbygrave It is currently impossible for some platforms to enable/disable history while there are pending drops.
(it will generate the wrong history table)
I don't mind this is a real limitation. I added a sanity check in Ddl-gneration
| // This could be probably fixed, by generating history table also with dropped columns | ||
| @Column(name = "`select`") | ||
| @Index(unique = true) | ||
| private String select; |
There was a problem hiding this comment.
Note to myself: This CAN be removed now, as long as previous version is under @history
This is a refactoring how DDL is generated, when quotedIdentifiers are used.
I've made an ETable entity, which uses a lot of reserved keywords.
This is something, that did not work for all platforms (especially MySql and SqlServer, as they use different quote chars than
")I checked if the generated migration scripts will work for H2/MySql/DB2/SqlServer and Hana.
It prepares ebean generate working migration scripts, if
allQuotedIdentifiers=trueis set - this probably will also be an other workaround forNONKEYWORDS=KEY,VALUEin H2To make this PR smaller, I've extracted some commits into separate PRs.
@rbygrave you can decide, if you either merge this PR or the smaller ones and I will update the PR next week