Tim Armstrong has posted comments on this change. Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec ......................................................................
Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/7245/3/be/src/exec/hdfs-scan-node-base.cc File be/src/exec/hdfs-scan-node-base.cc: Line 27: #include <tuple> Shouldn't this be in the header instead of the .cc? Line 776: const THdfsCompression::type& compression_type, bool skipped) { Parameter names don't match header Line 883: if (file_format == THdfsFileFormat::PARQUET) { One line comment explaining why Parquet is a special case. PS3, Line 886: ss << file_format << "(Skipped)" << "/" << compression_type << ":" : << it->second << " "; AVRO/SNAPPY(Skipped) I think reads better http://gerrit.cloudera.org:8080/#/c/7245/3/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: Line 257: const std::vector<THdfsCompression::type>& compression_type, bool filtered = false); Parameter names are inconsistent - skipped vs filtered. We should also document when 'skipped' should be set to true. -- To view, visit http://gerrit.cloudera.org:8080/7245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke <apha...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: anujphadke <apha...@cloudera.com> Gerrit-HasComments: Yes
