[Impala-ASF-CR] IMPALA-7942: Add query hints for cardinalities and selectivities

["Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>]

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 <sk...@163.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Fucun Chu <ch...@hotmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 07 Dec 2021 19:16:20 +0000
Gerrit-HasComments: Yes