Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-2373: Extrapolate row counts for HDFS tables. ......................................................................
Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/6840/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: PS2, Line 77: > Created in c'tor. There is no constructor that allows setting both values t Better, thanks. http://gerrit.cloudera.org:8080/#/c/6840/3/fe/src/main/java/org/apache/impala/catalog/View.java File fe/src/main/java/org/apache/impala/catalog/View.java: PS3, Line 118: tableStats_ = new TTableStats(-1); : tableStats_.setTotal_file_bytes(-1); I believe you don't need it now that you put the initialization in Table's ctor. http://gerrit.cloudera.org:8080/#/c/6840/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: PS3, Line 617: computeStats(Analyzer analyzer) { Significantly better. Thanks PS3, Line 655: statsNumRows_, totalBytes_ There are class members, why are you passing them as parameter? Is this function called somewhere else? PS3, Line 673: are ignored "because...". You may want to briefly mention why this is the case. PS3, Line 683: extrapolatedNumRows_ = tbl_.getExtrapolatedNumRows(totalBytes); This doesn't necessarily belong in this function. I'd move it to L654 since at this point totalBytes_ has been computed. http://gerrit.cloudera.org:8080/#/c/6840/3/tests/metadata/test_explain.py File tests/metadata/test_explain.py: PS3, Line 127: 50 Hm, I would expect this to be '100' since the query doesn't filter any partitions and '50' if the query was something like "select .. where p = 1". PS3, Line 131: nit: extra space -- To view, visit http://gerrit.cloudera.org:8080/6840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I972c8a03ed70211734631a7dc9085cb33622ebc4 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes
