Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18023 )
Change subject: IMPALA-7942: Add query hints for cardinalities and selectivities ...................................................................... Patch Set 3: (15 comments) Thanks for working on this functionality! http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@10 PS3, Line 10: this maybe lead nit: this may lead http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@34 PS3, Line 34: format nit: formats http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@45 PS3, Line 45: -1 isn't second 0.1? Based on IMPALA-7601 http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@48 PS3, Line 48: is been set nit: has been set / is set http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@48 PS3, Line 48: need ensure nit: need to ensure http://gerrit.cloudera.org:8080/#/c/18023/3//COMMIT_MSG@49 PS3, Line 49: maybe does not take effect as you : expected. nit: might not take the expected effect http://gerrit.cloudera.org:8080/#/c/18023/1/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/18023/1/fe/src/main/cup/sql-parser.cup@3763 PS1, Line 3763: including http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/cup/sql-parser.cup@3260 PS3, Line 3260: computing nit: computed http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/cup/sql-parser.cup@3829 PS3, Line 3829: incliding nit: including http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/cup/sql-parser.cup@3831 PS3, Line 3831: not_like_predicate Could you please add a test for not like predicate predicate-selectivity-hint.test? http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java File fe/src/main/java/org/apache/impala/analysis/Predicate.java: http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java@77 PS3, Line 77: analyzer.addWarning("Selectivity hint is allowed for single column predicate"); nit: maybe add more information about this predicate in the warning. http://gerrit.cloudera.org:8080/#/c/18023/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/18023/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1556 PS1, Line 1556: > In my opinion, table rows is precise, unless table not stats or has corrupt I don't have a strong opinion here, just want to mention that table stats can be stale. Though in that case we can issue COMPUTE STATS, or update the stats manually via ALTER TABLE stmt. http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@5008 PS3, Line 5008: AnalyzesOk Why is it not an analysis error? http://gerrit.cloudera.org:8080/#/c/18023/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@5050 PS3, Line 5050: (0) Don't we allow zero as selectivity hint? E.g. at L5060 we say 'allowed value is [0,1]' http://gerrit.cloudera.org:8080/#/c/18023/3/testdata/workloads/functional-planner/queries/PlannerTest/predicate-selectivity-hint.test File testdata/workloads/functional-planner/queries/PlannerTest/predicate-selectivity-hint.test: http://gerrit.cloudera.org:8080/#/c/18023/3/testdata/workloads/functional-planner/queries/PlannerTest/predicate-selectivity-hint.test@643 PS3, Line 643: 2.87M Above we have 2.42M for 0.5 selectivity. -- To view, visit http://gerrit.cloudera.org:8080/18023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2776b9bbd878b8a21d9c866b400140a454f59e1b Gerrit-Change-Number: 18023 Gerrit-PatchSet: 3 Gerrit-Owner: wangsheng <sky...@163.com> Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Fucun Chu <chufu...@hotmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: wangsheng <sky...@163.com> Gerrit-Comment-Date: Tue, 07 Dec 2021 19:16:20 +0000 Gerrit-HasComments: Yes
