Support for Sybase ASE and Informix #349
Conversation
Closes cmu-db#285 Closes cmu-db#286 Closes cmu-db#287 Closes cmu-db#288 Closes cmu-db#289
There was a problem hiding this comment.
@anasY2 Thank you for submitting this. Please see my comments about your commits.
Is there a way for us to add this to our CI tests with a Docker installation of Informix/Sybase? It would be nice to know that we can maintain compatibility with these systems over the years.
There was a problem hiding this comment.
What changed in this file other than whitespace?
| @@ -77,7 +77,7 @@ protected List<Worker<? extends BenchmarkModule>> makeWorkersImpl() { | |||
| // LOADING FROM THE DATABASE IMPORTANT INFORMATION | |||
| // LIST OF USERS | |||
| Table t = this.getCatalog().getTable("USERTABLE"); | |||
| String userCount = SQLUtil.getMaxColSQL(this.workConf.getDatabaseType(), t, "ycsb_key"); | |||
| String userCount = SQLUtil.getMaxColSQL(this.workConf.getDatabaseType(), t, "YCSB_KEY"); | |||
There was a problem hiding this comment.
Is this change specific to Informix?
There was a problem hiding this comment.
It was for Sybase ASE as it was not working with smaller case.
| @@ -35,7 +35,8 @@ public enum DatabaseType { | |||
| DB2(true, false), | |||
| H2(true, false), | |||
| HSQLDB(false, false), | |||
| POSTGRES(false, false, true), | |||
| INFORMIX(false, false), | |||
| POSTGRES(false, false), | |||
| @@ -47,8 +48,9 @@ public enum DatabaseType { | |||
| SPANNER(false, true), | |||
| SQLAZURE(true, true, true), | |||
| SQLITE(true, false), | |||
| SQLSERVER(true, true, true), | |||
| TIMESTEN(true, false), | |||
| SQLSERVER(true, false), | |||
There was a problem hiding this comment.
Again, these changes seem unnecessary.
There was a problem hiding this comment.
Unnecessary case changes. Please revert.
There was a problem hiding this comment.
Please don't include this change. It's only for local dev debugging.
|
@apavlo I will correct the things you have mentioned and will make a new PR. |
No description provided.