You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by tomatophantastico <gi...@git.apache.org> on 2017/07/21 22:44:28 UTC

[GitHub] metamodel pull request #150: Feature/quote columns

GitHub user tomatophantastico opened a pull request:

    https://github.com/apache/metamodel/pull/150

    Feature/quote columns

    Working on a problem similar to  METAMODEL-1143 ,this is my solution for HSQLDB and select queries.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tomatophantastico/metamodel feature/quoteColumns

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metamodel/pull/150.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #150
    
----
commit ea4cf2df161d103a6f32d39e58d6aadc341f3695
Author: Jörg Unbehauen <jo...@unbehauen.net>
Date:   2016-10-17T03:07:31Z

    test with different capitalizations of tables and columns

commit 1c51bfe840296d1a3cc1a0485db90d277da089ff
Author: Jörg Unbehauen <jo...@unbehauen.net>
Date:   2016-11-13T19:57:00Z

    update testcase

commit a6b6f7b9bf8f7c3e33f6609f5e257f379281501f
Author: Jörg Unbehauen <jo...@unbehauen.net>
Date:   2016-11-16T13:41:31Z

    uncommented tests that need clarification

commit 891fcfddb89c774332d75aa66f9e49e948d7990d
Author: Jörg Unbehauen <jo...@unbehauen.net>
Date:   2017-03-16T16:47:14Z

    escape identifiers on hsqldb

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel issue #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:

    https://github.com/apache/metamodel/pull/150
  
    Patched (but I moved the Guava dependency to the jdbc module).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128901008
  
    --- Diff: core/pom.xml ---
    @@ -32,6 +32,10 @@ under the License.
     			<artifactId>slf4j-api</artifactId>
     		</dependency>
     		<dependency>
    +			<groupId>com.google.guava</groupId>
    +			<artifactId>guava</artifactId>
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by tomatophantastico <gi...@git.apache.org>.
Github user tomatophantastico commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128903712
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -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) {
    --- End diff --
    
    agreed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel issue #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:

    https://github.com/apache/metamodel/pull/150
  
    The change looks good to me, but I had a few review remarks that I'd prefer us taking care of first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by tomatophantastico <gi...@git.apache.org>.
Github user tomatophantastico commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128903688
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/HsqldbTest.java ---
    @@ -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);){
    +        
    +
    +        
    +        final JdbcDataContext dcon = new JdbcDataContext(connection);
    +        
    +        final String mixedcap = "MixedCapitalization";
    +        final String idCol = "Id";
    +        final String nameCol =  "name";
    +        final String emailCol = "EMAIL";
    +        final String createTable = "CREATE TABLE \"" + mixedcap + "\" (\""+idCol+"\" INTEGER, \""+nameCol+"\" LONGVARCHAR,  \""+emailCol+"\" LONGVARCHAR)";
    +        
    +        try(Statement stmt = connection.createStatement();){
    +          
    +          stmt.execute(createTable);
    +          
    +        }
    +        dcon.refreshSchemas();
    +        assertEquals(mixedcap, dcon.getDefaultSchema().getTable(0).getName()); 
    +        assertEquals(idCol, dcon.getDefaultSchema().getTable(0).getColumn(0).getName());
    +        
    +        dcon.query().from(mixedcap).select(idCol,nameCol,emailCol).execute();
    +        
    +//        Query query = new Query();
    +//        String subselect = "(select * from \""+mixedcap+"\")";
    +//        query.from(subselect);
    +//        query.select("test");
    +//        
    +//        dcon.executeQuery(query);
    --- End diff --
    
    removed it.
    
    This code should demonstrate some other issue i had, but it should better be placed in an other test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128900988
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/HsqldbTest.java ---
    @@ -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);){
    +        
    +
    +        
    +        final JdbcDataContext dcon = new JdbcDataContext(connection);
    +        
    +        final String mixedcap = "MixedCapitalization";
    +        final String idCol = "Id";
    +        final String nameCol =  "name";
    +        final String emailCol = "EMAIL";
    +        final String createTable = "CREATE TABLE \"" + mixedcap + "\" (\""+idCol+"\" INTEGER, \""+nameCol+"\" LONGVARCHAR,  \""+emailCol+"\" LONGVARCHAR)";
    +        
    +        try(Statement stmt = connection.createStatement();){
    +          
    +          stmt.execute(createTable);
    +          
    +        }
    +        dcon.refreshSchemas();
    +        assertEquals(mixedcap, dcon.getDefaultSchema().getTable(0).getName()); 
    +        assertEquals(idCol, dcon.getDefaultSchema().getTable(0).getColumn(0).getName());
    +        
    +        dcon.query().from(mixedcap).select(idCol,nameCol,emailCol).execute();
    +        
    +//        Query query = new Query();
    +//        String subselect = "(select * from \""+mixedcap+"\")";
    +//        query.from(subselect);
    +//        query.select("test");
    +//        
    +//        dcon.executeQuery(query);
    --- End diff --
    
    I would leave out / remove commented code


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by tomatophantastico <gi...@git.apache.org>.
Github user tomatophantastico commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128903693
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/HsqldbTest.java ---
    @@ -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);){
    --- End diff --
    
    beautified the test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/metamodel/pull/150


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128900965
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -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) {
    --- End diff --
    
    I would suggest `protected` scope instead of `public`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by tomatophantastico <gi...@git.apache.org>.
Github user tomatophantastico commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128903665
  
    --- Diff: core/pom.xml ---
    @@ -32,6 +32,10 @@ under the License.
     			<artifactId>slf4j-api</artifactId>
     		</dependency>
     		<dependency>
    +			<groupId>com.google.guava</groupId>
    +			<artifactId>guava</artifactId>
    --- End diff --
    
    i add it in an other pull request to core, so i added it here too, otherwise it would not compile.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metamodel pull request #150: Feature/quote columns

Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/150#discussion_r128900981
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/HsqldbTest.java ---
    @@ -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);){
    --- End diff --
    
    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)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---