Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9139 )
Change subject: IMPALA-6228: Control stats extrapolation via tbl prop. ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java: http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@549 PS1, Line 549: this > nit: remove "this" Good point. Done. http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@130 PS1, Line 130: // setting for a specific table. By default, tables do not have property set and rely > nit: ... the property ... Done http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1971 PS1, Line 1971: * Returns true stats extrapolation is enabled for this table, false otherwise. > nit: if Done http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1975 PS1, Line 1975: enableStatsExtrapolation > Current name sounds like it modifies state instead of tests state. how abou Done http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java File fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java: http://gerrit.cloudera.org:8080/#/c/9139/1/fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java@73 PS1, Line 73: actualExrtapNumRows > nit: spelling Done http://gerrit.cloudera.org:8080/#/c/9139/1/tests/metadata/test_stats_extrapolation.py File tests/metadata/test_stats_extrapolation.py: http://gerrit.cloudera.org:8080/#/c/9139/1/tests/metadata/test_stats_extrapolation.py@25 PS1, Line 25: class TestStatsExtrapolation(ImpalaTestSuite): > perhaps I've misread the diff, but is the impalad flag for extrapolation st You are correct, the impalad flag is not tested with an E2E test anymore. I was thinking that might not be necessary, but happy to add a new test if you prefer. -- To view, visit http://gerrit.cloudera.org:8080/9139 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie49597bf1b93b7572106abc620d91f199cba0cfd Gerrit-Change-Number: 9139 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 30 Jan 2018 20:25:01 +0000 Gerrit-HasComments: Yes
