Feature/quote columns #150
Feature/quote columns #150
Conversation
@@ -101,7 +101,7 @@ public String rewriteColumnType(ColumnType columnType, Integer columnSize) { | |||
return super.rewriteColumnType(columnType, columnSize); | |||
} | |||
|
|||
private boolean needsQuoting(String alias, String identifierQuoteString) { | |||
public boolean needsQuoting(String alias, String identifierQuoteString) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest protected
scope instead of public
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
@@ -417,4 +418,98 @@ public void testInterpretationOfNull() throws Exception { | |||
Connection conn = DriverManager.getConnection("jdbc:hsqldb:mem:interpretation_of_null", USERNAME, PASSWORD); | |||
JdbcTestTemplates.interpretationOfNulls(conn); | |||
} | |||
|
|||
public void testCapitalization() throws Exception{ | |||
try(Connection connection = DriverManager.getConnection("jdbc:hsqldb:mem:different_types_insert", USERNAME,PASSWORD);){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coding conventions/formatting rule: There should be a space between "try" and "(".
(same rule is relevant other places in the diff, this is just an example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautified the test
// query.from(subselect); | ||
// query.select("test"); | ||
// | ||
// dcon.executeQuery(query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave out / remove commented code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed it.
This code should demonstrate some other issue i had, but it should better be placed in an other test
@@ -32,6 +32,10 @@ under the License. | |||
<artifactId>slf4j-api</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>com.google.guava</groupId> | |||
<artifactId>guava</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only add the dependency where necesary. You are using guava in the JDBC module, so you should add it as a dependency there instead of the core
module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i add it in an other pull request to core, so i added it here too, otherwise it would not compile.
The change looks good to me, but I had a few review remarks that I'd prefer us taking care of first. |
fixed indentions added space between try and (
Patched (but I moved the Guava dependency to the jdbc module). |
Working on a problem similar to METAMODEL-1143 ,this is my solution for HSQLDB and select queries.