----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18766/#review36229 -----------------------------------------------------------
The patch looks pretty good for me. I leaved one comment. tajo-storage/src/main/java/org/apache/tajo/storage/CSVFile.java <https://reviews.apache.org/r/18766/#comment67170> Even though its computation cost may be trivial, this boolean expression will evaluated the number of rows x the number of columns times. So, in some cases, it may be costly. Instead of checking both boolean variables, we could set one pre-computed boolean variable. It would be more efficient. - Hyunsik Choi On March 5, 2014, 9:06 p.m., Jinho Kim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18766/ > ----------------------------------------------------------- > > (Updated March 5, 2014, 9:06 p.m.) > > > Review request for Tajo. > > > Bugs: TAJO-657 > https://issues.apache.org/jira/browse/TAJO-657 > > > Repository: tajo > > > Description > ------- > > RCFile should add stat for fine-grained progress indicator.(TAJO-589) > 'Actual Processed Bytes' is 'decompressed bytes + overhead' > > > Diffs > ----- > > > tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querytasks.jsp > 744c0148a18cd01ece8be71041984b3ac293a75c > tajo-storage/src/main/java/org/apache/tajo/storage/CSVFile.java > 6c1923c1427bdacbae4c66ca6d87ac1cbb39a6b7 > tajo-storage/src/main/java/org/apache/tajo/storage/rcfile/RCFile.java > 90d7f48e9294e5e51cd7bceda6188c89baeb01d1 > > tajo-storage/src/test/java/org/apache/tajo/storage/TestCompressionStorages.java > bec1556e2f8301d485438b93666da3b9ba5b58d3 > tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java > f4ce46bea980ce86fa9cf498ccaffc2ce5953c72 > > Diff: https://reviews.apache.org/r/18766/diff/ > > > Testing > ------- > > mvn clean install > real cluster > > > Thanks, > > Jinho Kim > >
