Lars Volker has posted comments on this change. Change subject: IMPALA-4815, IMPALA-4817, IMPALA-4819: Write and Read Parquet Statistics for remaining types ......................................................................
Patch Set 7: (4 comments) Thanks for the review. Please see PS8. http://gerrit.cloudera.org:8080/#/c/6563/6/be/src/exec/parquet-column-stats.cc File be/src/exec/parquet-column-stats.cc: Line 104: case TYPE_DECIMAL: > let's disable support. (there is a jira, but the number escapes me.) I disabled read support for CHAR columns. Should we also disable write support. Currently we write CHAR columns through ColumnWriter<StringValue>, so disabling them would require special handling in there. http://gerrit.cloudera.org:8080/#/c/6563/6/be/src/exec/parquet-column-stats.h File be/src/exec/parquet-column-stats.h: Line 105: private: > what if col_order == 0? Expanded the comment, though I had deemed this an implementation detail. Should it just say "col_order can be null"? Line 172: /// Returns the number of bytes needed to encode value 'v'. > move to -base and remove template params in .inl.h? Done http://gerrit.cloudera.org:8080/#/c/6563/6/be/src/exec/parquet-column-stats.inline.h File be/src/exec/parquet-column-stats.inline.h: Line 123: if (ParquetPlainEncoder::Decode(data, data + size, size, result) == -1) return false; > several function signatures in parquet-common.h look wrong (output args at Done -- To view, visit http://gerrit.cloudera.org:8080/6563 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3ef4a5d25a57c82577fd498d6d1c4297ecf39312 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
