Dan Hecht has posted comments on this change.

Change subject: IMPALA-4988: Add query option read_parquet_statistics
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7001/1/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

Line 632:     if (state_->query_options().read_parquet_statistics) {
okay to ignore, but consider putting this inside EvaluateStatsConjuncts(). 
You'd want the query option to always force stats to be ignored by that 
function, not just particular to this callsite, right? (I understand there's 
currently only one callsite, but you get my point).


http://gerrit.cloudera.org:8080/#/c/7001/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 243: read_parquet_statistics
consider renaming this so the "parquet" comes first like all the other parquet 
options, which makes them easier to find. maybe parquet_statistics_filtering, 
to mirror parquet_dictionary_filtering?


http://gerrit.cloudera.org:8080/#/c/7001/1/testdata/workloads/functional-query/queries/QueryTest/parquet_stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet_stats.test:

Line 452: # query option.
how does this actually verify that stats were disabled? maybe check the stats 
counter.


-- 
To view, visit http://gerrit.cloudera.org:8080/7001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I427f7fde40d0f4b703751e40f3c2109a850643f7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Wood <[email protected]>
Gerrit-HasComments: Yes

Reply via email to