[jira] [Comment Edited] (CALCITE-1553) Forget to change qualifier when converting INVERVAL_YEAR literal

["Julian Hyde (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-1553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15773454#comment-15773454 ] 

Julian Hyde edited comment on CALCITE-1553 at 12/23/16 6:45 PM:
----------------------------------------------------------------

I don't see a problem. I ran your example and it gave the correct result, {{date '2009-12-31'}}.

{noformat}
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 3b90848..63c3e52 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -2765,6 +2765,12 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) {
         "DATE NOT NULL");
   }
 
+  @Test public void testDatePlusMonthInterval() {
+    tester.checkScalar("date '2008-12-31' + interval '1' year",
+        "2009-12-31",
+        "DATE NOT NULL");
+  }
+
   @Test public void testNotEqualsOperator() {
     tester.setFor(SqlStdOperatorTable.NOT_EQUALS);
     tester.checkBoolean("1<>1", Boolean.FALSE);
{noformat}

Maybe you misunderstand how interval literals are represented internally: YEAR and MONTH intervals are represented in months, all others are represented in seconds. Normalizing units makes things easier for Calcite internals.

Can you describe the benefits of exposing {{SqlNodeToRexConverter}}? The cost (albeit a fairly minor one) is that it is yet another API that we have to maintain compatibility in. We already expose {{SqlRexConvertletTable}} (see CALCITE-250), so that gives you control over how function and operator calls are translated.


was (Author: julianhyde):
I don't see a problem. I ran your example and it gave the correct result, {{date '2009-12-31'}}.

{noformat}
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 3b90848..63c3e52 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -2765,6 +2765,12 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) {
         "DATE NOT NULL");
   }
 
+  @Test public void testDatePlusMonthInterval() {
+    tester.checkScalar("date '2008-12-31' + interval '1' year",
+        "2009-12-31",
+        "DATE NOT NULL");
+  }
+
   @Test public void testNotEqualsOperator() {
     tester.setFor(SqlStdOperatorTable.NOT_EQUALS);
     tester.checkBoolean("1<>1", Boolean.FALSE);
{noformat}

Maybe you misunderstand how interval literals are represented internally: YEAR and MONTH intervals are represented in months, all others are represented in seconds. Normalizing units makes things easier for Calcite internals.

Can you describe the benefits of exposing `SqlNodeToRexConverter`? The cost (albeit a fairly minor one) is that it is yet another API that we have to maintain compatibility in. We already expose `SqlRexConvertletTable` (see CALCITE-250), so that gives you control over how function and operator calls are translated.

> Forget to change qualifier when converting INVERVAL_YEAR literal
> ----------------------------------------------------------------
>
>                 Key: CALCITE-1553
>                 URL: https://issues.apache.org/jira/browse/CALCITE-1553
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Baofeng Zhang
>            Assignee: Julian Hyde
>
> If I execute this SQL:
> {code}SELECT date '2008-12-31' + INTERVAL '1' YEAR{code}
> {{INTERVAL '1' YEAR}} is converted into {{INTERVAL '12' YEAR}} inside {{SqlNodeToRexConverterImpl.convertLiteral}} method, which forgets to change qualifier to MONTH.
> Besides {{INTERVAL_YEAR_MONTH}}, there maybe other {{SqlTypeFamily}} have the same problem. 
> In addition, can we expose {{SqlNodeToRexConverter}} in {{SqlToRelConverter}} to let developer to plugin their own converter?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)