-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20125/#review40744
-----------------------------------------------------------

Ship it!


+1

I deeply reviewed the patch. This patch fixes several bugs, and the patch 
includes enough unit tests. Ship it.

- Hyunsik Choi


On April 18, 2014, 11:57 a.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20125/
> -----------------------------------------------------------
> 
> (Updated April 18, 2014, 11:57 a.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: TAJO-741
>     https://issues.apache.org/jira/browse/TAJO-741
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> I found a bug for GreedyHeuristicJoinOrderAlgorithm as follows:
> 1. Table Schema
> create external table table1 (id int, name text, score float, type text) 
> using csv with ('csvfile.delimiter'='|') location 
> 'hdfs://server01:9010/tajo/warehouse/table1' ;
> 
> create external table table3 (id int, name text, score float, type text) 
> using csv with ('csvfile.delimiter'='|') location 
> 'hdfs://localhost:9010/tajo/warehouse/table3' ;
> 
> create external table table4 (id int, name text, score float, type text) 
> using csv with ('csvfile.delimiter'='|') location 
> 'hdfs://localhost:9010/tajo/warehouse/table4' ;
> 2. Table Data
> 2.1 table1
> 1|name1-1|1.1|a
> 2|name1-2|2.3|b
> 3|name1-3|3.4|c
> 4|name1-4|4.5|d
> 5|name1-5|5.6|e
> 
> 2.2 table3
> 1|name3-1|0.1|a
> 2|name3-2|0.2|b
> 3|name3-3|0.3|b
> 
> 2.3 table4
> 1|name4-1|22.3|a
> 2|name4-2|23.4|b
> 3|name4-3|24.5|cc
> 5|name4-4|25.6|ee
> 6|name4-5|31.1|ff
> 7|name4-6|32.3|gg
> 3. Query
> select a.name, c.name, a.type, c.type from table1 a join table3 b on (a.id = 
> b.id) join table4 c on (b.id = c.id) where a.type =  c.type;
> 4. Expected Result
> name,  name,  type,  type
> -------------------------------
> name1-1,  name4-1,  a,  a
> name1-2,  name4-2,  b,  b
> 5. Active Result
> name,  name,  type,  type
> -------------------------------
> name1-1,  name4-1,  a,  a
> name1-2,  name4-2,  b,  b
> name1-3,  name4-3,  c,  cc
> I found that default.a.type (TEXT) = default.c.type (TEXT) is in initiative 
> plan result and FPD result. 
> But after executing GreedyHeuristicJoinOrderAlgorithm, it disappeared in 
> logical plan.
> 
> 
> Diffs
> -----
> 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/EvalTreeUtil.java
>  0966ee0 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java
>  5b43c5b 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java
>  ffa95fb 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
>  2da1f4b 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/HashFullOuterJoinExec.java
>  dc06bd9 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/HashJoinExec.java
>  0084031 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/HashLeftOuterJoinExec.java
>  6d57961 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
>  f1daec9 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java
>  d0fde4f 
>   
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java
>  6704230 
>   
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
>  6dda611 
>   
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestPlannerUtil.java
>  746f6fb 
>   
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinBroadcast.java
>  1b68577 
>   
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
>  65187c6 
>   
> tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testJoinWithMultipleJoinQual1.sql
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testJoinWithMultipleJoinQual2.sql
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testJoinWithMultipleJoinQual3.sql
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testJoinWithMultipleJoinQual4.sql
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testJoinWithMultipleJoinQual1.result
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testJoinWithMultipleJoinQual2.result
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testJoinWithMultipleJoinQual3.result
>  PRE-CREATION 
>   
> tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testJoinWithMultipleJoinQual4.result
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20125/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>

Reply via email to