----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33136/#review80902 -----------------------------------------------------------
Ship it! Difficult to tell the rationale behind each change, but all of the changes seem to make sense. I noticed that you had to move to the version of ProjectRemoveRule.isTrivial with a boolean parameter. That was temporary, and was removed shortly afterwards. See CALCITE-575 and CALCITE-577. Unavoidable, I suppose. At a later point, consider renaming Drill's RelNodes and Rules to match Calcite's new naming scheme. Strictly optional of course. - Julian Hyde On April 18, 2015, 8:34 p.m., Jinfeng Ni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33136/ > ----------------------------------------------------------- > > (Updated April 18, 2015, 8:34 p.m.) > > > Review request for drill and Aman Sinha. > > > Repository: drill-git > > > Description > ------- > > Drill currently uses a forked Optiq (Renamed to Calcite) version, dated back > in July 2014. The forked version has 10-20 Drill specific patches. However, > we did not rebase the forked version onto the on-going Calcite release. As > such, Drill misses some bug fixes/new feature development on Calcite side. > > This patch is trying to rebase the Drill's forked version onto Calcite > release 1.0. (More precisely, on commit of CALCITE-603, when this rebasing > work started). > > Most of changes happen due to Calcite's package structure / renaming (See > CALCITE-296]. > > Some Drill specific changes: > > 1. Provide a Drill specifc RelDataTypeSystem, to support decimal with > precision/scale up to 38. > 2. Modify Drill's parser, to allow * in Compound Identifier. > 3. Provide Drill specific FilterJoinRule, to enforce Drill only support > equal-join in JOIN operator. > 4. Modify Drill costing comparison, such that the costing oder is a total > order when compare different plans. > 5. Modify costing estimation for Drill Project operator. > 6. Use a ProjectRemove rule, such that it will honor parent's output field > name. > 7. Modify Calcite's Frameworks/planner interface, such that Drill will use > validatedRowType to construct a top-level project, to ensure the final output > field is what the query specified. (Calcite could inject "$F0", or "$EXPR0 > into converted RelNode tree, in Sql2RelConverter) > 8. Fix couple of Drill unit test cases, since the expected result by query > semantics are not fixed. > 9. Some type-related to Drill Sql operators. > > Some impact of such rebasing. > 1. TPCH Q16, or query with NOT IN predicate involving NULLABLE column could > hit CanNotPlanException. > > The previous plan for Q16, although return the correct result, is not valid, > when the column is nullable. See DRILL-1957 OR CALCITE-373. > > 2. Plan changed in some TPCH queries, which may cause timeout for Q5 in TPCH > scale factor 100 run. > We probably need continue refine the costing estimation formula, especially > for Join operator. > > > Diffs > ----- > > common/pom.xml 525b533 > contrib/storage-hive/core/pom.xml 9bd6293 > exec/java-exec/pom.xml f5313ca > exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl 50d8c20 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java > 3b3aa1a > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillProjectRelBase.java > 7cf98cd > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java > bbe7cf3 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillCostBase.java > 87a1ea3 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillFilterJoinRules.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java > 14ea894 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java > fcfced2 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java > 29175e5 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > 84a0b51 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashJoinPrel.java > f63057f > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java > 7bd48c8 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java > a17a604 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java > 5ee502d > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java > 1cce6a5 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java > c347bef > exec/jdbc-all/pom.xml b369aed > exec/jdbc/pom.xml b4ec758 > pom.xml f6bcd91 > > Diff: https://reviews.apache.org/r/33136/diff/ > > > Testing > ------- > > Unit testcase clean run. (except for disabled testcases. See DRILL-2630, > DRILL-2761, etc) > > > Thanks, > > Jinfeng Ni > >
