Pooja Nilangekar has posted comments on this change. Change subject: IMPALA-5061: Populate null_count in parquet::statistics ......................................................................
Patch Set 5: (11 comments) http://gerrit.cloudera.org:8080/#/c/7058/4//COMMIT_MSG Commit Message: Line 10: value is encountered by parquet table writer. The value is written > nit typo Done http://gerrit.cloudera.org:8080/#/c/7058/4/be/src/exec/hdfs-parquet-table-writer.cc File be/src/exec/hdfs-parquet-table-writer.cc: Line 197: if (page_stats_base_ != nullptr) page_stats_base_->UpdateNullCount(); > nit: single line Done http://gerrit.cloudera.org:8080/#/c/7058/2/be/src/exec/parquet-column-stats.h File be/src/exec/parquet-column-stats.h: Line 79: > For frequently called functions it's often better to return a bool instead Done http://gerrit.cloudera.org:8080/#/c/7058/4/be/src/exec/parquet-column-stats.h File be/src/exec/parquet-column-stats.h: PS4, Line 63: ; > remove Done PS4, Line 105: ing > column? Done PS4, Line 116: r'. > nit: lowercase Done http://gerrit.cloudera.org:8080/#/c/7058/4/be/src/exec/parquet-column-stats.inline.h File be/src/exec/parquet-column-stats.inline.h: Line 32: template <typename T> > It looks like this will fit into one line in the header. If so, please move Done Line 186: has_values_ = true; > You could also change the interface of Update() to take a min and max param Done Line 205: template <> > If you add a "int64_t count" parameter to the UpdateNullCount method, you c Done http://gerrit.cloudera.org:8080/#/c/7058/4/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: Line 324: > How about moving this to L330, initialize it from stats.null_count and then Done Line 480: is set for columns with null values.""" > Please remove the trailing whitespace. You can also set-up git in a way tha Done -- To view, visit http://gerrit.cloudera.org:8080/7058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c49a63af84c2234f0633be63206cb52eb7e8ebb Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Pooja Nilangekar <[email protected]> Gerrit-HasComments: Yes
