----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18556/#review35591 -----------------------------------------------------------
The bug fix looks nice for me. I leave some comments about more comments and correctness. tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java <https://reviews.apache.org/r/18556/#comment66231> Could you comment the purpose of this code? Actually, so far, we have been lazy for commenting. Later, we need to make an effort to add more comprehensive comments. tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java <https://reviews.apache.org/r/18556/#comment66232> It may be wrong in some cases. groupby, sort, limit, and projection can be stacked on the JoinNode. In those cases, this if-condition will pass this workaround code. If you use PlannerUtil.findMostBottomNode, it will be true in all cases because the most bottom join node is the subject to require the shuffle. - Hyunsik Choi On Feb. 27, 2014, 12:01 p.m., Jung JaeHwa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18556/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 12:01 p.m.) > > > Review request for Tajo. > > > Bugs: TAJO-620 > https://issues.apache.org/jira/browse/TAJO-620 > > > Repository: tajo > > > Description > ------- > > A join query can cause IndexOutOfBoundsException if one of tables is empty. > The reproduction condition is very straightforward. We should fix it. > > > Diffs > ----- > > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/benchmark/TPCH.java > 2e12b1d > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java > 0d3f95e > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java > 3a95724 > > tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testInnerJoinWithEmptyTable.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testInnerJoinWithEmptyTable.result > PRE-CREATION > > Diff: https://reviews.apache.org/r/18556/diff/ > > > Testing > ------- > > mvn clean install > > > Thanks, > > Jung JaeHwa > >
