impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5742: De-allocate buffer in parquet-reader on exit
Date Thu, 03 Aug 2017 04:53:29 GMT
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 <henry@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message