----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21902/#review44708 -----------------------------------------------------------
Hi Jaehwa, thanks for your contribution. In overall, the patch looks good to me. I left only one comment below. Thanks! tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/HashAggregateExec.java <https://reviews.apache.org/r/21902/#comment79184> I have a question. In my opinion, the tuple of null datums should be returned whenever the next() is called when there aren't any grouping keys and grouped entries. However, in this logic, the tuple of null datums is returned only one time when the variable 'finished' is false. Would you add some more explanation for this logic? - Jihoon Son On May 30, 2014, 1:04 p.m., Jung JaeHwa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21902/ > ----------------------------------------------------------- > > (Updated May 30, 2014, 1:04 p.m.) > > > Review request for Tajo. > > > Bugs: TAJO-840 > https://issues.apache.org/jira/browse/TAJO-840 > > > Repository: tajo > > > Description > ------- > > If there is no data, tsql will print as follows: > > default> select * from table1; > id, name, score, type > ------------------------------- > 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 > > default> select count(*) from table1 where id = 10; > Progress: 100%, response time: 0.231 sec > ?count > ------------------------------- > (0 rows, 0.231 sec, 0 B selected) > But it need to print result detailedly as follows: > ?count > ------------------------------- > 0 > ------------------------------- > (1 rows, 0.231 sec, 0 B selected) > Because if users implement some applications with TajoClient or TajoJDBC, > they can't understand easy that a query finished successfully or a query > finished unsuccessfully. > > > Diffs > ----- > > > tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbyHashAggregationExec.java > 8daad0b > > tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java > fd79725 > > tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/HashAggregateExec.java > c87e01a > > tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortAggregateExec.java > 9a415d1 > > tajo-core/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java > 3a2e79f > tajo-core/src/main/java/org/apache/tajo/worker/Task.java f065951 > tajo-core/src/test/java/org/apache/tajo/engine/query/TestGroupByQuery.java > 1263bbe > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData1.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData10.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData11.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData12.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData2.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData3.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData4.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData5.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData6.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData7.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData8.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestGroupByQuery/testGroupByWithNullData9.sql > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData1.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData10.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData11.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData12.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData2.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData3.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData4.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData5.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData6.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData7.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData8.result > PRE-CREATION > > tajo-core/src/test/resources/results/TestGroupByQuery/testGroupByWithNullData9.result > PRE-CREATION > > Diff: https://reviews.apache.org/r/21902/diff/ > > > Testing > ------- > > mvn clean install > > > Thanks, > > Jung JaeHwa > >
