Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5742: De-allocate buffer in parquet-reader on exit
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7572/1/be/src/util/parquet-reader.cc
File be/src/util/parquet-reader.cc:

PS1, Line 291: (double)
Not your change, but these C style casts are against the google style guide we 
follow. Probably a good idea to leave a TODO at the top of the file to take 
care of this during spring cleaning.


PS1, Line 297:      << "  Metadata size: " << metadata_len << "(" << 
(metadata_len / (double)file_len)
             :      << ")" << endl
             :      << "  Total page header size: " << total_page_header_size 
<< "("
             :      << (total_page_header_size / (double)file_len) << ")" << 
endl;
             :   ss << "  Column compressed size: " << 
total_compressed_data_size << "("
             :      << (total_compressed_data_size / (double)file_len) << ")" 
<< endl;
             :   ss << "  Column uncompressed size: " << 
total_uncompressed_data_size << "("
             :      << compression_ratio << ")" << endl;
             :   for (int i = 0; i < column_byte_sizes.size(); ++i) {
             :     ss << "    "
             :        << "Col " << i << ": " << column_byte_sizes[i] << "("
             :        << (column_byte_sizes[i] / (double)file_len) << ")" << 
endl;
nit: Why change this formatting? The way it already was seems easier to read.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iedf163f858b42d2ca63a7a65d6e457539de59ab9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to