You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by haohui <gi...@git.apache.org> on 2017/07/19 21:45:54 UTC

[GitHub] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

GitHub user haohui opened a pull request:

    https://github.com/apache/flink/pull/4373

    [FLINK-6429] [table] Bump up Calcite version to 1.13.

    This PR bumps upgrades Calcite from 1.12 to 1.13. It includes fixes for CALCITE-1884 and FLINK-7159. We can probably separate it to smaller PR as well.

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

    $ git pull https://github.com/haohui/flink FLINK-6429

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

    https://github.com/apache/flink/pull/4373.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 #4373
    
----
commit aa0e69d795f4b1d718bfa9b1ffc0f62f78243b43
Author: Haohui Mai <wh...@apache.org>
Date:   2017-07-19T21:34:37Z

    [FLINK-6429] Bump Calcite version to 1.13.

----


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130827249
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    Yes, I'm also curious why this plan would be generated. AFAIK, the `DataStreamWindowJoin` is very strict about the join predicates it accepts. It needs two range predicates with time indicator attributes on both sides. If the computation is pushed into the `DataStreamCalc` the time indicator attribute should be materialized and no longer be identified as a time indicator. 


---
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] flink issue #4373: [FLINK-6429] [table] Bump up Calcite version to 1.13.

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

    https://github.com/apache/flink/pull/4373
  
    I have did some investigation of the test failure of **JoinITCase.testJoinWithExpressionPreds** and would like to share my findings and solutions for your reference. The cause of this issue is that for preserved expressions, in **PushProjector#createProjectRefsAndExprs**, the column names corresponding to them will be the operator names of the expressions. For example for expression **a - 1** in the test case, the  column corresponding to it will be **-**. I think this behavior is not expected and have copied **PushProjector** from calcite and made some changes to it (line 507). Please refer to [here](https://github.com/dianfu/flink/commit/efa9641e0bd395a3679b0d496b60e3d42aa7b832) for more information.
    
    For the files copied from calcite, all files can be removed except **SqlTimestampAddFunction** and **AuxiliaryConverter**.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130789546
  
    --- Diff: flink-libraries/flink-table/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java ---
    @@ -1039,7 +1039,7 @@ public boolean argumentMustBeScalar(int ordinal) {
     		new SqlCollectionTableOperator("TABLE", SqlModality.RELATION);
     
     	public static final SqlOverlapsOperator OVERLAPS =
    --- End diff --
    
    Do you think it is a good idea to separate it into another PR? It might simplify the reviewing process.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130794757
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/time.scala ---
    @@ -355,26 +355,36 @@ case class TemporalOverlaps(
           rightT: RexNode,
           relBuilder: FlinkRelBuilder)
         : RexNode = {
    -    // leftT = leftP + leftT if leftT is an interval
    -    val convLeftT = if (isTimeInterval(leftTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, leftP, leftT)
    -      } else {
    -        leftT
    -      }
    -    // rightT = rightP + rightT if rightT is an interval
    -    val convRightT = if (isTimeInterval(rightTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, rightP, rightT)
    -      } else {
    -        rightT
    -      }
    -    // leftT >= rightP
    -    val leftPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convLeftT, rightP)
    -    // rightT >= leftP
    -    val rightPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convRightT, leftP)
    +    val convLeftT = convertOverlapsEnd(relBuilder, leftP, leftT, leftTemporal.resultType)
    --- End diff --
    
    The problem is that SQL semantic shifted from Calcite 1.12 to Calcite 1.13. Do you think it is a good idea to disable the relevant tests temporarily and then bring it back momentarily?


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131079109
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/time.scala ---
    @@ -355,26 +355,36 @@ case class TemporalOverlaps(
           rightT: RexNode,
           relBuilder: FlinkRelBuilder)
         : RexNode = {
    -    // leftT = leftP + leftT if leftT is an interval
    -    val convLeftT = if (isTimeInterval(leftTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, leftP, leftT)
    -      } else {
    -        leftT
    -      }
    -    // rightT = rightP + rightT if rightT is an interval
    -    val convRightT = if (isTimeInterval(rightTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, rightP, rightT)
    -      } else {
    -        rightT
    -      }
    -    // leftT >= rightP
    -    val leftPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convLeftT, rightP)
    -    // rightT >= leftP
    -    val rightPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convRightT, leftP)
    +    val convLeftT = convertOverlapsEnd(relBuilder, leftP, leftT, leftTemporal.resultType)
    --- End diff --
    
    Good point. I'm fine with fixing this with this PR.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r129239292
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/time.scala ---
    @@ -355,26 +355,36 @@ case class TemporalOverlaps(
           rightT: RexNode,
           relBuilder: FlinkRelBuilder)
         : RexNode = {
    -    // leftT = leftP + leftT if leftT is an interval
    -    val convLeftT = if (isTimeInterval(leftTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, leftP, leftT)
    -      } else {
    -        leftT
    -      }
    -    // rightT = rightP + rightT if rightT is an interval
    -    val convRightT = if (isTimeInterval(rightTemporal.resultType)) {
    -        relBuilder.call(SqlStdOperatorTable.DATETIME_PLUS, rightP, rightT)
    -      } else {
    -        rightT
    -      }
    -    // leftT >= rightP
    -    val leftPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convLeftT, rightP)
    -    // rightT >= leftP
    -    val rightPred = relBuilder.call(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, convRightT, leftP)
    +    val convLeftT = convertOverlapsEnd(relBuilder, leftP, leftT, leftTemporal.resultType)
    --- End diff --
    
    Can you open a separate PR for this? It should be part of FLINK-7159. I think we can get this in before this PR.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130801549
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    In the original test, the calculation of **proctime** is located when doing **Join**. While currently, it's pushed down to the **DataStremaCalc**. It seems that this is not correct?


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131512787
  
    --- Diff: flink-libraries/flink-table/src/main/java/org/apache/calcite/rel/rules/PushProjector.java ---
    @@ -0,0 +1,864 @@
    +/*
    + * 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.calcite.rel.rules;
    +
    --- End diff --
    
    Yes,  you are right. We should make sure it is a Calcite bug and will be fixed in the next release. 


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r129240196
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/sql/CorrelateTest.scala ---
    @@ -43,7 +43,7 @@ class CorrelateTest extends TableTestBase {
             term("invocation", "func1($cor0.c)"),
             term("function", func1.getClass.getCanonicalName),
             term("rowType",
    -             "RecordType(INTEGER a, BIGINT b, VARCHAR(2147483647) c, VARCHAR(2147483647) f0)"),
    +             "RecordType(INTEGER a, BIGINT b, VARCHAR(65536) c, VARCHAR(65536) f0)"),
    --- End diff --
    
    It seems that Calcite has defined an upper limit for strings and ignores our size. Can you also update this in `FlinkTypeSystem` and the comment in `FlinkTypeFactory#createSqlType`?


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131302891
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    I think it's easy to prevent the time indicator push-down in our side. Calcite 1.3 changed the `ProjectJoinTransposeRule.INSTANCE` to `new ProjectJoinTransposeRule(         PushProjector.ExprCondition.TRUE, RelFactories.LOGICAL_BUILDER);` instead of `new ProjectJoinTransposeRule(PushProjector.ExprCondition.FALSE, RelFactories.LOGICAL_BUILDER);`.  
    
    In order to change the default behavior of the rule, the only thing we need to do is to create a new `ProjectJoinTransposeRule` with a custom `PushProjector.ExprCondition` which filters prevent time indicator rex nodes.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131094518
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    Yes, you're of course right @twalthr. This is a valid rewrite and I agree with you're analysis. We have to prevent the push-down of predicates that include time indicator fields. The `DataStreamWindowJoin` needs the offsets to compute the correct window boundaries.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131346038
  
    --- Diff: flink-libraries/flink-table/src/main/java/org/apache/calcite/rel/rules/PushProjector.java ---
    @@ -0,0 +1,864 @@
    +/*
    + * 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.calcite.rel.rules;
    +
    --- End diff --
    
    Can you add a comment here when we can remove this file again (corresponding CALCITE-XXXX issue?).


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r129237304
  
    --- Diff: flink-libraries/flink-table/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java ---
    @@ -1039,7 +1039,7 @@ public boolean argumentMustBeScalar(int ordinal) {
     		new SqlCollectionTableOperator("TABLE", SqlModality.RELATION);
     
     	public static final SqlOverlapsOperator OVERLAPS =
    --- End diff --
    
    I think we can remove this file for now and rely on Calcite 1.13 implementation. We don't need it anymore.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130829080
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    I think this issue may be caused by CALCITE-1753, still investigating the changes in that JIRA. 


---
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] flink issue #4373: [FLINK-6429] [table] Bump up Calcite version to 1.13.

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

    https://github.com/apache/flink/pull/4373
  
    Thanks for the update @haohui. I think this PR is in a good shape now. I will go over the changes again and merge this. Unfortunately, the new findbugs plugin causes the build to fail. We have to wait until this is fixed.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r130789743
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/sql/CorrelateTest.scala ---
    @@ -43,7 +43,7 @@ class CorrelateTest extends TableTestBase {
             term("invocation", "func1($cor0.c)"),
             term("function", func1.getClass.getCanonicalName),
             term("rowType",
    -             "RecordType(INTEGER a, BIGINT b, VARCHAR(2147483647) c, VARCHAR(2147483647) f0)"),
    +             "RecordType(INTEGER a, BIGINT b, VARCHAR(65536) c, VARCHAR(65536) f0)"),
    --- End diff --
    
    Will do.


---
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] flink issue #4373: [FLINK-6429] [table] Bump up Calcite version to 1.13.

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

    https://github.com/apache/flink/pull/4373
  
    @dianfu thanks for the investigation! It turns out that it no longer requires copying {{PushProjector}} after rebasing to the latest tip. Updated the PR to address the issue.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r129237531
  
    --- Diff: flink-libraries/flink-table/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java ---
    @@ -1869,13 +1869,23 @@ private RexNode convertOver(Blackboard bb, SqlNode node) {
     			// Walk over the tree and apply 'over' to all agg functions. This is
     			// necessary because the returned expression is not necessarily a call
     			// to an agg function. For example, AVG(x) becomes SUM(x) / COUNT(x).
    +
    --- End diff --
    
    I think we can remove this file for now and rely on Calcite 1.13 implementation. We don't need it anymore.


---
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] flink pull request #4373: [FLINK-6429] [table] Bump up Calcite version to 1....

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

    https://github.com/apache/flink/pull/4373#discussion_r131085265
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/JoinTest.scala ---
    @@ -55,12 +55,13 @@ class JoinTest extends TableTestBase {
               unaryNode(
                 "DataStreamCalc",
                 streamTableNode(1),
    -            term("select", "a", "b", "proctime")
    +            term("select", "a", "b", "-(proctime, 3600000) AS -",
    --- End diff --
    
    @fhueske Actually, from an optimizer perspective this behavior is correct. But we don't want to push down predicates with time indicators. The time indicators are not materialized in this case as the materialization happens before optimization.


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