[FLINK-4068] [tableAPI] Move constant computations out of code-generated #2102

wuchong · 2016-06-15T09:31:12Z

The ReduceExpressionsRule rule can reduce constant expressions and replacing them with the corresponding constant. We have ReduceExpressionsRule.CALC_INSTANCE in both DATASET_OPT_RULES and DATASET_OPT_RULES, but it dose not take effect. Because it require the planner have an executor to evaluate the constant expressions. This PR does this, and resolve FLINK-4068.

And some tests added.

…ted functions.

fhueske · 2016-06-15T09:54:33Z

Thanks for the PR @wuchong! I will have a look at it soon :-)

fhueske · 2016-06-15T19:56:05Z

...libraries/flink-table/src/test/scala/org/apache/flink/api/scala/batch/sql/SelectITCase.scala

@@ -146,4 +148,21 @@ class SelectITCase(
    tEnv.sql(sqlQuery)
  }

+  @Test
+  def testConstantReduce(): Unit = {


I think the assertion of this this test should not check the name of the generate Flink operator.
Instead I propose the following:

split the translate() method into an optimize() method that generates the optimized RelNode tree and a translate() method that translates into a DataSet / DataStream program.

make the optimize() method private[flink] and therefore accessible from a unit test

add a BatchTableEnvironmentTest and a StreamTableEnvironmentTest which check that the optimized RelNode tree contains reduced expressions.

👍 It's a good idea. I will try it later. And the CI throws cannot translate call AS... error, I will figure it out today.

fhueske · 2016-06-15T20:04:02Z

This @wuchong, your approach looks good. I also found that the ReduceExpressionRules had no effect due to the missing RexExecutor.

However, it seems that several tests of ExpressionITCase are failing with this change. You can verify that the PR does not break the build by locally running mvn clean install.

In addition, the added test should be changed as sketched in the comment. Please let me know if you have questions.

Thanks, Fabian

wuchong · 2016-06-16T15:26:40Z

After introducing RexExecutor which make ReduceExpressionRules taking effect , many errors occurred.

The cannot translate call AS($t0, $t1) is a Calcite bug I think, and I created a related issue : CALCITE-1295.

We should replace L69&L73 to relBuilder.call(SqlStdOperatorTable.CONCAT, l, cast) otherwise will throw the following exception. Because calcite have no plus(String, String) method.

java.lang.RuntimeException: while resolving method 'plus[class java.lang.String, class java.lang.String]' in class class org.apache.calcite.runtime.SqlFunctions

at org.apache.calcite.linq4j.tree.Types.lookupMethod(Types.java:345)
at org.apache.calcite.linq4j.tree.Expressions.call(Expressions.java:442)
at org.apache.calcite.adapter.enumerable.RexImpTable$BinaryImplementor.implement(RexImpTable.java:1640)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementCall(RexImpTable.java:854)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementNullSemantics(RexImpTable.java:843)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementNullSemantics0(RexImpTable.java:756)
at org.apache.calcite.adapter.enumerable.RexImpTable.access$900(RexImpTable.java:181)
at org.apache.calcite.adapter.enumerable.RexImpTable$3.implement(RexImpTable.java:411)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateCall(RexToLixTranslator.java:535)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate0(RexToLixTranslator.java:507)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:222)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate0(RexToLixTranslator.java:472)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:222)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:217)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateList(RexToLixTranslator.java:700)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateProjects(RexToLixTranslator.java:192)
at org.apache.calcite.rex.RexExecutorImpl.compile(RexExecutorImpl.java:80)
at org.apache.calcite.rex.RexExecutorImpl.compile(RexExecutorImpl.java:59)
at org.apache.calcite.rex.RexExecutorImpl.reduce(RexExecutorImpl.java:118)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressionsInternal(ReduceExpressionsRule.java:544)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressions(ReduceExpressionsRule.java:455)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressions(ReduceExpressionsRule.java:438)
at org.apache.calcite.rel.rules.ReduceExpressionsRule$CalcReduceExpressionsRule.onMatch(ReduceExpressionsRule.java:350)
at org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:213)
at org.apache.calcite.plan.volcano.VolcanoPlanner.findBestExp(VolcanoPlanner.java:838)
at org.apache.calcite.tools.Programs$RuleSetProgram.run(Programs.java:334)
at org.apache.flink.api.table.BatchTableEnvironment.translate(BatchTableEnvironment.scala:250)
at org.apache.flink.api.java.table.BatchTableEnvironment.toDataSet(BatchTableEnvironment.scala:146)
at org.apache.flink.api.java.batch.table.ExpressionsITCase.testCom

The following error is when we convert Trim to RexNode, we use a Integer to represent "LEADING", "TRAILING", "BOTH". Instead we should use SqlTrimFunction.Flag. But I haven't found how to write SqlTrimFunction.Flag into a RexNode.

java.lang.ClassCastException: java.lang.Integer cannot be cast to org.apache.calcite.sql.fun.SqlTrimFunction$Flag

at org.apache.calcite.adapter.enumerable.RexImpTable$TrimImplementor.implement(RexImpTable.java:1448)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementCall(RexImpTable.java:854)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementNullSemantics(RexImpTable.java:843)
at org.apache.calcite.adapter.enumerable.RexImpTable.implementNullSemantics0(RexImpTable.java:756)
at org.apache.calcite.adapter.enumerable.RexImpTable.access$900(RexImpTable.java:181)
at org.apache.calcite.adapter.enumerable.RexImpTable$3.implement(RexImpTable.java:411)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateCall(RexToLixTranslator.java:535)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate0(RexToLixTranslator.java:507)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:222)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate0(RexToLixTranslator.java:472)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:222)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:217)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateList(RexToLixTranslator.java:700)
at org.apache.calcite.adapter.enumerable.RexToLixTranslator.translateProjects(RexToLixTranslator.java:192)
at org.apache.calcite.rex.RexExecutorImpl.compile(RexExecutorImpl.java:80)
at org.apache.calcite.rex.RexExecutorImpl.compile(RexExecutorImpl.java:59)
at org.apache.calcite.rex.RexExecutorImpl.reduce(RexExecutorImpl.java:118)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressionsInternal(ReduceExpressionsRule.java:544)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressions(ReduceExpressionsRule.java:455)
at org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressions(ReduceExpressionsRule.java:438)
at org.apache.calcite.rel.rules.ReduceExpressionsRule$CalcReduceExpressionsRule.onMatch(ReduceExpressionsRule.java:350)
at org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:213)
at org.apache.calcite.plan.volcano.VolcanoPlanner.findBestExp(VolcanoPlanner.java:838)
at org.apache.calcite.tools.Programs$RuleSetProgram.run(Programs.java:334)
at org.apache.flink.api.table.BatchTableEnvironment.translate(BatchTableEnvironment.scala:250)
at org.apache.flink.api.java.table.BatchTableEnvironment.toDataSet(BatchTableEnvironment.scala:146)
at org.apache.flink.api.java.batch.table.ExpressionsITCase.testComplexExpression(ExpressionsITCase.java:197)

And some other errors I didn't figure out , looks like calcite bugs.

twalthr · 2016-06-17T08:02:52Z

Regarding 3.: I already have some code in a branch that introduces symbols instead of the "Integer hack" we are currently using, but I haven't finished it so far. I can solve this issue for you.

wuchong · 2016-06-17T08:24:16Z

@twalthr That sounds great ! Thank you .

twalthr · 2016-08-29T14:07:06Z

@wuchong I have added symbols recently. So functions like TRIM are now called correctly. If you like you could update this PR.

wuchong · 2016-08-30T03:38:07Z

Thanks @twalthr . This issue is more complex than I expected before. I will try to update this PR in next days.

wuchong · 2016-09-08T09:01:44Z

As I mentioned above, the cannot translate call AS($t0, $t1) error because of a Calcite bug. The CALCITE-1297 fixed it , but not yet released. I will wait for Calcite 1.9 released and start working on it.

[FLINK-4068] [tableAPI] Move constant computations out of code-genera…

786653b

…ted functions.

fhueske reviewed Jun 15, 2016
View reviewed changes

wuchong closed this Sep 27, 2016

wuchong mentioned this pull request Sep 28, 2016

[FLINK-4068] [table] Move constant computations out of code-generated #2560

Closed

rmetzger added the component=API/TableSQL label Mar 14, 2019

flinkbot added component=TableSQL/API and removed component=API/TableSQL labels Mar 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-4068] [tableAPI] Move constant computations out of code-generated #2102

[FLINK-4068] [tableAPI] Move constant computations out of code-generated #2102

wuchong commented Jun 15, 2016

fhueske commented Jun 15, 2016

fhueske Jun 15, 2016

wuchong Jun 16, 2016

fhueske commented Jun 15, 2016

wuchong commented Jun 16, 2016

twalthr commented Jun 17, 2016

wuchong commented Jun 17, 2016

twalthr commented Aug 29, 2016

wuchong commented Aug 30, 2016

wuchong commented Sep 8, 2016

[FLINK-4068] [tableAPI] Move constant computations out of code-generated #2102

[FLINK-4068] [tableAPI] Move constant computations out of code-generated #2102

Conversation

wuchong commented Jun 15, 2016

fhueske commented Jun 15, 2016

fhueske Jun 15, 2016

Choose a reason for hiding this comment

wuchong Jun 16, 2016

Choose a reason for hiding this comment

fhueske commented Jun 15, 2016

wuchong commented Jun 16, 2016

twalthr commented Jun 17, 2016

wuchong commented Jun 17, 2016

twalthr commented Aug 29, 2016

wuchong commented Aug 30, 2016

wuchong commented Sep 8, 2016