[ 
https://issues.apache.org/jira/browse/BEAM-5072?focusedWorklogId=131569&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-131569
 ]

ASF GitHub Bot logged work on BEAM-5072:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 06/Aug/18 20:28
            Start Date: 06/Aug/18 20:28
    Worklog Time Spent: 10m 
      Work Description: apilloud closed pull request #6139: 
[BEAM-5072][SQL]Remove RelCollationTraitDef configuration in BeamQueryPlanner
URL: https://github.com/apache/beam/pull/6139
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/BeamQueryPlanner.java
 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/BeamQueryPlanner.java
index 3d11ebf43a4..bdee34a7a96 100644
--- 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/BeamQueryPlanner.java
+++ 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/BeamQueryPlanner.java
@@ -31,7 +31,6 @@
 import org.apache.calcite.plan.RelTraitDef;
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.prepare.CalciteCatalogReader;
-import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.RelRoot;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.sql.SqlNode;
@@ -76,8 +75,7 @@
     final SchemaPlus schema = connection.getRootSchema();
     final SchemaPlus defaultSchema = JdbcDriver.getDefaultSchema(connection);
 
-    final ImmutableList<RelTraitDef> traitDefs =
-        ImmutableList.of(ConventionTraitDef.INSTANCE, 
RelCollationTraitDef.INSTANCE);
+    final ImmutableList<RelTraitDef> traitDefs = 
ImmutableList.of(ConventionTraitDef.INSTANCE);
 
     final CalciteCatalogReader catalogReader =
         new CalciteCatalogReader(
diff --git 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/JdbcDriver.java
 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/JdbcDriver.java
index fe7d572f547..f3ce0016165 100644
--- 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/JdbcDriver.java
+++ 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/JdbcDriver.java
@@ -24,6 +24,8 @@
 import com.google.common.base.Function;
 import java.sql.Connection;
 import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import org.apache.beam.sdk.extensions.sql.impl.parser.impl.BeamSqlParserImpl;
@@ -42,7 +44,10 @@
 import org.apache.calcite.jdbc.Driver;
 import org.apache.calcite.plan.RelOptPlanner;
 import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelTraitDef;
+import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.rules.CalcRemoveRule;
+import org.apache.calcite.rel.rules.SortRemoveRule;
 import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.tools.RuleSet;
@@ -92,6 +97,15 @@ public Void apply(RelOptPlanner planner) {
               }
             }
             planner.removeRule(CalcRemoveRule.INSTANCE);
+            planner.removeRule(SortRemoveRule.INSTANCE);
+
+            List<RelTraitDef> relTraitDefs = new 
ArrayList<>(planner.getRelTraitDefs());
+            planner.clearRelTraitDefs();
+            for (RelTraitDef def : relTraitDefs) {
+              if (!(def instanceof RelCollationTraitDef)) {
+                planner.addRelTraitDef(def);
+              }
+            }
             return null;
           }
         });
diff --git 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamRuleSets.java
 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamRuleSets.java
index 6c13da8b1fe..24d72c1fd67 100644
--- 
a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamRuleSets.java
+++ 
b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamRuleSets.java
@@ -54,7 +54,6 @@
 import org.apache.calcite.rel.rules.ProjectToCalcRule;
 import org.apache.calcite.rel.rules.PruneEmptyRules;
 import org.apache.calcite.rel.rules.SortProjectTransposeRule;
-import org.apache.calcite.rel.rules.SortRemoveRule;
 import org.apache.calcite.rel.rules.UnionEliminatorRule;
 import org.apache.calcite.rel.rules.UnionToDistinctRule;
 import org.apache.calcite.tools.RuleSet;
@@ -120,7 +119,8 @@
           // AggregateReduceFunctionsRule.INSTANCE,
 
           // remove unnecessary sort rule
-          SortRemoveRule.INSTANCE,
+          // https://issues.apache.org/jira/browse/BEAM-5073
+          // SortRemoveRule.INSTANCE,
 
           // prune empty results rules
           PruneEmptyRules.AGGREGATE_INSTANCE,
diff --git 
a/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/interpreter/BeamSqlFnExecutorTestBase.java
 
b/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/interpreter/BeamSqlFnExecutorTestBase.java
index 2d4e9b19c6e..fd7cdc59284 100644
--- 
a/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/interpreter/BeamSqlFnExecutorTestBase.java
+++ 
b/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/interpreter/BeamSqlFnExecutorTestBase.java
@@ -30,7 +30,6 @@
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelTraitDef;
 import org.apache.calcite.plan.volcano.VolcanoPlanner;
-import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
 import org.apache.calcite.rex.RexBuilder;
@@ -70,8 +69,7 @@ public static void prepare() {
             .build();
 
     SchemaPlus schema = Frameworks.createRootSchema(true);
-    final ImmutableList<RelTraitDef> traitDefs =
-        ImmutableList.of(ConventionTraitDef.INSTANCE, 
RelCollationTraitDef.INSTANCE);
+    final ImmutableList<RelTraitDef> traitDefs = 
ImmutableList.of(ConventionTraitDef.INSTANCE);
     FrameworkConfig config =
         Frameworks.newConfigBuilder()
             .parserConfig(SqlParser.configBuilder().setLex(Lex.MYSQL).build())


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 131569)
    Time Spent: 1h 40m  (was: 1.5h)

> SortRelNode is wrongly removed
> ------------------------------
>
>                 Key: BEAM-5072
>                 URL: https://issues.apache.org/jira/browse/BEAM-5072
>             Project: Beam
>          Issue Type: Bug
>          Components: dsl-sql
>            Reporter: Rui Wang
>            Assignee: Rui Wang
>            Priority: Major
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> ORDER BY query generates LogicalSort in logical plan, but it will be removed 
> wrongly during optimization by the SortRemoveRule



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to