> On April 27, 2014, 11:28 p.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java, > > line 1 > > <https://reviews.apache.org/r/20741/diff/1/?file=569055#file569055line1> > > > > Do you think it would be good to separate the cost model change from > > the new optimizer plan support for HashAgg/HashJoin, and put them into two > > different JIRA? > > Aman Sinha wrote: > Probably. I could split those out if other reviewers also feel the same.
After discussion, decided to keep the changes together since it would be some effort to split them apart. > On April 27, 2014, 11:28 p.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillCostBase.java, > > line 182 > > <https://reviews.apache.org/r/20741/diff/1/?file=569047#file569047line182> > > > > Here, why do we add rowCount together with CPU, IO, NETWORK? Are they > > in the same scale (having the same unit)? > > > > I thought cpu, io, network will all computed from rowCount. But when we > > compare two costs, rowCount would not play directly. > > Aman Sinha wrote: > They have the same data type (double). I suppose logically it makes > sense to compare row counts with another and combine the rest in one group. > I can do that. Modified such that {cpu, io, network} are compared as a set and rowCount is compared separately. - Aman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20741/#review41572 ----------------------------------------------------------- On April 26, 2014, 1:54 a.m., Aman Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20741/ > ----------------------------------------------------------- > > (Updated April 26, 2014, 1:54 a.m.) > > > Review request for drill, Jacques Nadeau and Jinfeng Ni. > > > Repository: drill-git > > > Description > ------- > > 1. Added DrillCostBase that includes distribution (network) cost. > 2. Added cost formulas for computeSelfCost() for various Exchange operators, > joins, aggregations etc. > 3. Added Prels for HashJoin, HashAggregate and generate plans for those. > 4. Added exchange Prels: BroadcastExchangePrel and a new type called > HashToMergeExchangePrel. > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java > 7e3b63d > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashToMergeExchange.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java > 98892c0 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillFilterRelBase.java > 955729b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillProjectRelBase.java > cf3d188 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillScanRelBase.java > b370352 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillScreenRelBase.java > 51ed442 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillCostBase.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelOptCost.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelOptCostFactory.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > 1492a28 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/BroadcastExchangePrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/DrillDistributionTrait.java > b75fb40 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/DrillDistributionTraitDef.java > c2ebb7a > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/FilterPrel.java > 0fc3abd > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashJoinPrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashJoinPrule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashToMergeExchangePrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashToRandomExchangePrel.java > e5c9661 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/MergeJoinPrel.java > 978a531 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/MergeJoinPrule.java > 8298e50 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java > e6e99c0 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/ScanPrel.java > a945129 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/SingleMergeExchangePrel.java > d9431cc > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/SortPrel.java > 344be4e > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java > c2880da > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrule.java > a561a61 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/TopNPrel.java > e981a45 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionExchangePrel.java > f89cbaa > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > 8892a8f > exec/java-exec/src/test/java/org/apache/drill/TestTpchSingleMode.java > 1ccb65c > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java > ba067e2 > exec/java-exec/src/test/resources/join/hj_exchanges.json PRE-CREATION > exec/java-exec/src/test/resources/queries/tpch/01.sql f33416f > > Diff: https://reviews.apache.org/r/20741/diff/ > > > Testing > ------- > > Existing unit tests. Some TPCH tests show memory leaks but that is not > directly related to the costing changes...however those leaks would have to > be resolved before enabling the new plans. However, I wanted to get this > review request out to get feedback. > > > Thanks, > > Aman Sinha > >
