drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Julian Hyde" <jul...@hydromatic.net>
Subject Re: Review Request 33136: DRILL-1384: Rebase Drill's forked Optiq library onto Calcite release 1.0.
Date Tue, 21 Apr 2015 04:27:35 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message