Juan Yu has posted comments on this change.

Change subject: IMPALA-1886/IMPALA-2154: Add support for multi-stream bz2/gzip 
compressed files.
......................................................................


Patch Set 14:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/2219/14/be/src/util/codec.h
File be/src/util/codec.h:

Line 117:   //
> /// (here and below)
Done


Line 120:   /// file. Use ProcessBlock() when decompressing blocks, for 
example, hdfd-avro-scanner.
> HdfsAvroScanner
Done


Line 130:   ///   stream_end: if decompressor consumed all input and reached 
end of compressed stream.
> I think we should remove the "if decompressor consumed all input" condition
Done


http://gerrit.cloudera.org:8080/#/c/2219/14/be/src/util/decompress.cc
File be/src/util/decompress.cc:

Line 89:   *stream_end = false;
> move this to L91 inside the loop.  See comment at line 113 for why.
Done


Line 113:       if (stream_.avail_in == 0) *stream_end = true;
> As mentioned elsewhere, let's simplify this and just uncondtionally set *st
Done


Line 342:   *stream_end = false;
> same comments as above
Done


Line 352:       if (stream_.avail_in == 0) *stream_end = true;
> and here
Done


http://gerrit.cloudera.org:8080/#/c/2219/14/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

Line 226: decompressed
> compressed
Done


http://gerrit.cloudera.org:8080/#/c/2219/14/testdata/workloads/functional-query/queries/DataErrorsTest/hdfs-scan-node-errors.test
File 
testdata/workloads/functional-query/queries/DataErrorsTest/hdfs-scan-node-errors.test:

Line 194: decompressed
> this should say "compressed" file.  From the users perspective, it's the co
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/2219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icbe617d03a69953f0bf3aa0f7c30d34bc612f9f8
Gerrit-PatchSet: 14
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.5.0_5.7.0
Gerrit-Owner: Juan Yu <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-HasComments: Yes

Reply via email to