You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by ankit2711 <gi...@git.apache.org> on 2015/10/16 23:08:45 UTC

[GitHub] metamodel pull request: Fix METAMODEL-198.

GitHub user ankit2711 opened a pull request:

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

    Fix METAMODEL-198.

     This fixes the issue but the changes are done in the FormatHelper. Few more test cases added for Timestamp values. 
    Trying to apply a fix for Timestamp operand in the DefaultQueryRewriter
    does get overriden by the logic written in the FormatHelper which is
    called by super.rewriteFilterItems(FilterItem). 
    The FormatHelper again detects based on the columnType and changes the
    values to be Date losing the nanoseconds.

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

    $ git pull https://github.com/ankit2711/metamodel master

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

    https://github.com/apache/metamodel/pull/58.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 #58
    
----
commit ac10d56e18c0a51cc57e6e8da1694aacf6a28f56
Author: ankit2711 <ak...@gmail.com>
Date:   2015-10-16T21:06:41Z

    Fix METAMODEL-198. This fixes the issue but the changes are done in the
    FormatHelper. 
    Trying to apply a fix for Timestamp operand in the DefaultQueryRewriter
    does get overriden by the logic written in the FormatHelper which is
    called by super.rewriteFilterItems(FilterItem). 
    The FormatHelper again detects based on the columnType and changes the
    values to be Date losing the nanoseconds.

----


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42322120
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/JdbcTestTemplates.java ---
    @@ -633,4 +634,84 @@ public void run(UpdateCallback callback) {
                 dataContext.executeUpdate(new DropTable(defaultSchema, testTableName));
             }
         }
    +
    +    public static void timestampValueInsertSelect(Connection conn) throws Exception {
    +        assertNotNull(conn);
    +
    +        try {
    +            // clean up, if nescesary
    +            conn.createStatement().execute("DROP TABLE test_table");
    +        } catch (SQLException e) {
    +            // do nothing
    +        }
    +
    +        assertFalse(conn.isReadOnly());
    +
    +        JdbcDataContext dc = new JdbcDataContext(conn);
    +        final Schema schema = dc.getDefaultSchema();
    +        final Timestamp timestamp1 = Timestamp.valueOf("2015-10-16 16:33:33.456");
    +        final Timestamp timestamp2 = Timestamp.valueOf("2015-10-16 16:33:34.683");
    --- End diff --
    
    That's exactly why I want to use the JDBC escape syntax so the DB can make such conversions itself.


---
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: Fix METAMODEL-198.

Posted by ankit2711 <gi...@git.apache.org>.
Github user ankit2711 commented on the pull request:

    https://github.com/apache/metamodel/pull/58#issuecomment-149153838
  
    The fixes are also done in another pull request from Kasper. That pull request also fixes some issues with Timestamp while insertion time. Closing this PR then.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#issuecomment-148921871
  
    Also a note on testing - if we add it as a scenario in JdbcTestTemplates then we can immediately verify it with all the automatic tests that can run for Derby, H2, HSQLDB, SQLite. If it works for those then I always feel a lot more comforted that we have something with a certain broader applicability.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42318415
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/JdbcTestTemplates.java ---
    @@ -633,4 +634,84 @@ public void run(UpdateCallback callback) {
                 dataContext.executeUpdate(new DropTable(defaultSchema, testTableName));
             }
         }
    +
    +    public static void timestampValueInsertSelect(Connection conn) throws Exception {
    +        assertNotNull(conn);
    +
    +        try {
    +            // clean up, if nescesary
    +            conn.createStatement().execute("DROP TABLE test_table");
    +        } catch (SQLException e) {
    +            // do nothing
    +        }
    +
    +        assertFalse(conn.isReadOnly());
    +
    +        JdbcDataContext dc = new JdbcDataContext(conn);
    +        final Schema schema = dc.getDefaultSchema();
    +        final Timestamp timestamp1 = Timestamp.valueOf("2015-10-16 16:33:33.456");
    +        final Timestamp timestamp2 = Timestamp.valueOf("2015-10-16 16:33:34.683");
    --- End diff --
    
    These are just milliseconds I think? Shouldn't we be adding nanoseconds to the test, or else we're not getting into the real precision gotchas here?


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42322079
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -139,13 +143,24 @@ public String rewriteFilterItem(FilterItem item) {
                         }
                     }
     
    -                FilterItem replacementFilterItem = new FilterItem(item.getSelectItem(), item.getOperator(), elements);
    +                FilterItem replacementFilterItem = new FilterItem(selectItem, item.getOperator(), elements);
                     return super.rewriteFilterItem(replacementFilterItem);
                 }
             }
             return super.rewriteFilterItem(item);
         }
    -    
    +
    +    protected String rewriteTimestamp(FilterItem item) {
    --- End diff --
    
    Now this method does a rewrite of a whole filter item and not just the timestamp literal. I was suggesting to make something that would make the burden of overriding as little as possible, so that you would only need to do like this:
    
    ```
    protected String rewriteTimestamp(Timestamp timestamp) {
      return MyStrangeTimestampFormat.format(timestamp);
    } 
    ```


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42322102
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -139,13 +143,24 @@ public String rewriteFilterItem(FilterItem item) {
                         }
                     }
     
    -                FilterItem replacementFilterItem = new FilterItem(item.getSelectItem(), item.getOperator(), elements);
    +                FilterItem replacementFilterItem = new FilterItem(selectItem, item.getOperator(), elements);
                     return super.rewriteFilterItem(replacementFilterItem);
                 }
             }
             return super.rewriteFilterItem(item);
         }
    -    
    +
    +    protected String rewriteTimestamp(FilterItem item) {
    +        final OperatorType operator = item.getOperator();
    +        final SelectItem selectItem = item.getSelectItem();
    +        final Object operand = item.getOperand();
    +        StringBuilder sb = new StringBuilder();
    +        sb.append(selectItem.getSameQueryAlias(false));
    +        FilterItem.appendOperator(sb, operand, operator);
    +        sb.append("TIMESTAMP \'" + operand.toString() + "\'");
    --- End diff --
    
    To be specific, I would change this line to:
    
    ```
    sb.append("{ts '" +  operand.toString() + "'}");
    ```


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42319647
  
    --- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/JdbcTestTemplates.java ---
    @@ -633,4 +634,84 @@ public void run(UpdateCallback callback) {
                 dataContext.executeUpdate(new DropTable(defaultSchema, testTableName));
             }
         }
    +
    +    public static void timestampValueInsertSelect(Connection conn) throws Exception {
    +        assertNotNull(conn);
    +
    +        try {
    +            // clean up, if nescesary
    +            conn.createStatement().execute("DROP TABLE test_table");
    +        } catch (SQLException e) {
    +            // do nothing
    +        }
    +
    +        assertFalse(conn.isReadOnly());
    +
    +        JdbcDataContext dc = new JdbcDataContext(conn);
    +        final Schema schema = dc.getDefaultSchema();
    +        final Timestamp timestamp1 = Timestamp.valueOf("2015-10-16 16:33:33.456");
    +        final Timestamp timestamp2 = Timestamp.valueOf("2015-10-16 16:33:34.683");
    --- End diff --
    
    This is hard because not all DB drivers claim to have the complete fractions parts. If the 6 digit number is placed for the nanos then in Derby. H2 and HSQL only first 3 digits are stored. 


---
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: Fix METAMODEL-198.

Posted by ankit2711 <gi...@git.apache.org>.
Github user ankit2711 commented on the pull request:

    https://github.com/apache/metamodel/pull/58#issuecomment-148960339
  
    Modified the fix to be now in DefaultQueryRewriter, added tests which work for Derby, H2 and HSQL. For SQLLite the timestamp insertions also do not store the milliseconds so skipped for now. Nothing is done in FormatHelper specific to Timestamp now.


---
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: Fix METAMODEL-198.

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

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


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42322087
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -139,13 +143,24 @@ public String rewriteFilterItem(FilterItem item) {
                         }
                     }
     
    -                FilterItem replacementFilterItem = new FilterItem(item.getSelectItem(), item.getOperator(), elements);
    +                FilterItem replacementFilterItem = new FilterItem(selectItem, item.getOperator(), elements);
                     return super.rewriteFilterItem(replacementFilterItem);
                 }
             }
             return super.rewriteFilterItem(item);
         }
    -    
    +
    +    protected String rewriteTimestamp(FilterItem item) {
    +        final OperatorType operator = item.getOperator();
    +        final SelectItem selectItem = item.getSelectItem();
    +        final Object operand = item.getOperand();
    +        StringBuilder sb = new StringBuilder();
    +        sb.append(selectItem.getSameQueryAlias(false));
    +        FilterItem.appendOperator(sb, operand, operator);
    +        sb.append("TIMESTAMP \'" + operand.toString() + "\'");
    --- End diff --
    
    Did you try with the official JDBC escape syntax? I bet that has a lot better general applicability than this format...


---
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: Fix METAMODEL-198.

Posted by ankit2711 <gi...@git.apache.org>.
Github user ankit2711 commented on the pull request:

    https://github.com/apache/metamodel/pull/58#issuecomment-149042383
  
    Done all. 


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#issuecomment-148908260
  
    The build is failing so I doubt this is a good approach, but I'd like to give some input and thoughts... I was trying to read up on JDBC spec to understand what can be done here.
    
    In section 13.4.2 of the JDBC 4.1 spec [1] it shows that proper SQL statement escape would be
    
    date:  ```{d 'yyyy-mm-dd'```
    time: ```{t 'hh:mm:ss'}```
    timestamp: ```{ts 'yyyy-mm-dd hh:mm:ss.f . . .'}```
    
    I'm always quite skeptical to see if the JDBC drivers out there _actually_ implement such details of the JDBC spec. Often I have found that implementations are not spot on in that regard. But I think the above could form the basis of a good default. We could override the default in case of deviating JDBC drivers.
    
    Our normal way of handling such deviations is to not rely on the FormatHelper (because it is generic and context-free) but rather rely on the ```IQueryRewriter``` interface, which actually has the method ```rewriteFilterItem(...)``` method. There's a hierarchy of classes that implement the interface and the default implementation (which most deviating implementations extend from) does delegate eventually to FormatHelper. But that's what we should change IMO. So instead of changing FormatHelper I would change ```DefaultQueryRewriter```.
    
    I would also want to cover this in all of our JDBC integration tests. Those aren't part of the Travis build but once we have a good implementation going then I am happy to test at least on the usual suspect databases such as PostgreSQL, MySQL, MS SQL Server, DB2 and Oracle.
    
    [1] http://download.oracle.com/otn-pub/jcp/jdbc-4_1-mrel-spec/jdbc4.1-fr-spec.pdf


---
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: Fix METAMODEL-198.

Posted by ankit2711 <gi...@git.apache.org>.
Github user ankit2711 commented on the pull request:

    https://github.com/apache/metamodel/pull/58#issuecomment-148913788
  
    The build was failing because some tests were added with the use of the below constructor 
    new Timestamp(long milliseconds) and then assertion were applied on the Timestamp in string. The constructor use with long milliseconds involves timeszone and hence the assertions failed. 
    
    Have tested on Postgres and Oracle when using [TIMESTAMP 'yyyy-mm-dd hh:mm:ss.nnnnnn'] in the query then the database does the conversion with respect to the format used in the timestamp column of that DB. The format of the timestamp column in Oracle had the storage of timestamp value in a format like this 16-OCT-15 04.33.34.646685000 PM.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42319638
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -106,14 +107,22 @@ private boolean needsQuoting(String alias, String identifierQuoteString) {
         public String rewriteFilterItem(FilterItem item) {
             Object operand = item.getOperand();
             if (operand != null) {
    +            final SelectItem selectItem = item.getSelectItem();
                 if (operand instanceof String) {
                     String str = (String) operand;
                     // escape single quotes
                     if (str.indexOf('\'') != -1) {
                         str = escapeQuotes(str);
    -                    FilterItem replacementFilterItem = new FilterItem(item.getSelectItem(), item.getOperator(), str);
    +                    FilterItem replacementFilterItem = new FilterItem(selectItem, item.getOperator(), str);
                         return super.rewriteFilterItem(replacementFilterItem);
                     }
    +            } else if (operand instanceof Timestamp) {
    +                final OperatorType operator = item.getOperator();
    +                StringBuilder sb = new StringBuilder();
    +                sb.append(selectItem.getSameQueryAlias(false));
    +                FilterItem.appendOperator(sb, operand, operator);
    +                sb.append("TIMESTAMP \'" + operand.toString() + "\'");
    --- End diff --
    
    Done.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#issuecomment-148914106
  
    Yes but I would love to have it solved in as generically possible way as we can. If the JDBC spec says that JDBC drivers should take into account the format of ```{ts 'yyyy-mm-dd hh:mm:ss.f . . .'}``` then that is probably the sanest to go for as the default. I would expect (but need to verify) that then the Oracle and PostgreSQL drivers can convert that expression to whatever is needed in the physical query fired from within the driver.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42318408
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DefaultQueryRewriter.java ---
    @@ -106,14 +107,22 @@ private boolean needsQuoting(String alias, String identifierQuoteString) {
         public String rewriteFilterItem(FilterItem item) {
             Object operand = item.getOperand();
             if (operand != null) {
    +            final SelectItem selectItem = item.getSelectItem();
                 if (operand instanceof String) {
                     String str = (String) operand;
                     // escape single quotes
                     if (str.indexOf('\'') != -1) {
                         str = escapeQuotes(str);
    -                    FilterItem replacementFilterItem = new FilterItem(item.getSelectItem(), item.getOperator(), str);
    +                    FilterItem replacementFilterItem = new FilterItem(selectItem, item.getOperator(), str);
                         return super.rewriteFilterItem(replacementFilterItem);
                     }
    +            } else if (operand instanceof Timestamp) {
    +                final OperatorType operator = item.getOperator();
    +                StringBuilder sb = new StringBuilder();
    +                sb.append(selectItem.getSameQueryAlias(false));
    +                FilterItem.appendOperator(sb, operand, operator);
    +                sb.append("TIMESTAMP \'" + operand.toString() + "\'");
    --- End diff --
    
    Would probably be a good idea to make a protected method "rewriteTimestamp(Timestamp)" so that we can easily overwrite just that in any deviating dialects.


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42318399
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DerbyQueryRewriter.java ---
    @@ -0,0 +1,50 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.metamodel.jdbc.dialects;
    +
    +import java.sql.Timestamp;
    +
    +import org.apache.metamodel.jdbc.JdbcDataContext;
    +import org.apache.metamodel.query.FilterItem;
    +import org.apache.metamodel.query.OperatorType;
    +
    +/**
    + * Query rewriter for Derby
    + */
    +public class DerbyQueryRewriter extends DefaultQueryRewriter {
    +
    +    public DerbyQueryRewriter(JdbcDataContext dataContext) {
    +        super(dataContext);
    +    }
    +
    +    @Override
    +    public String rewriteFilterItem(FilterItem item) {
    +        final Object operand = item.getOperand();
    +        if (operand instanceof Timestamp) {
    +            final OperatorType operator = item.getOperator();
    +            StringBuilder sb = new StringBuilder();
    +            sb.append(item.getSelectItem().getSameQueryAlias(false));
    +            FilterItem.appendOperator(sb, operand, operator);
    +            sb.append("TIMESTAMP ( \'" + operand.toString() + "\') ");
    --- End diff --
    
    Is this different from the parent somehow? I can't see a difference, but may be missing a detail somewhere?


---
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: Fix METAMODEL-198.

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

    https://github.com/apache/metamodel/pull/58#discussion_r42319497
  
    --- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/dialects/DerbyQueryRewriter.java ---
    @@ -0,0 +1,50 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.metamodel.jdbc.dialects;
    +
    +import java.sql.Timestamp;
    +
    +import org.apache.metamodel.jdbc.JdbcDataContext;
    +import org.apache.metamodel.query.FilterItem;
    +import org.apache.metamodel.query.OperatorType;
    +
    +/**
    + * Query rewriter for Derby
    + */
    +public class DerbyQueryRewriter extends DefaultQueryRewriter {
    +
    +    public DerbyQueryRewriter(JdbcDataContext dataContext) {
    +        super(dataContext);
    +    }
    +
    +    @Override
    +    public String rewriteFilterItem(FilterItem item) {
    +        final Object operand = item.getOperand();
    +        if (operand instanceof Timestamp) {
    +            final OperatorType operator = item.getOperator();
    +            StringBuilder sb = new StringBuilder();
    +            sb.append(item.getSelectItem().getSameQueryAlias(false));
    +            FilterItem.appendOperator(sb, operand, operator);
    +            sb.append("TIMESTAMP ( \'" + operand.toString() + "\') ");
    --- End diff --
    
    Yes it expects the format like TIMESTAMP ( ' operand value ' ), so the round braces are needed else it does not work. I guess timestamp queries on Derby might be broken on master.


---
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.
---