Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11057 )
Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and TIMESTAMP_MICROS from Parquet ...................................................................... Patch Set 18: (2 comments) http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc@518 PS16, Line 518: We need to get min stats. : statsField = ColumnStatsReader::StatsField::MIN; : } else if (fn_name == "gt" || fn_name == "ge") { : // We need to get max stats. : statsField = ColumnStatsReader::StatsField::MAX; : } else { : DCHECK(false) << "Unsupported function name for statistics evaluation: " << fn_name; : c > I see what you mean, but I did not want add TimestampHelper to the simple R I still prefer to hide that logic in ReadFromThrift(), but feel free to leave it as it is. http://gerrit.cloudera.org:8080/#/c/11057/18/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11057/18/be/src/exec/hdfs-parquet-scanner.cc@516 PS18, Line 516: statsField nit: stats_field -- To view, visit http://gerrit.cloudera.org:8080/11057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3 Gerrit-Change-Number: 11057 Gerrit-PatchSet: 18 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Mon, 05 Nov 2018 17:17:10 +0000 Gerrit-HasComments: Yes
