wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18829 )

Change subject: IMPALA-7942 (part 1): Add query hints for table cardinalities
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18829/5/fe/src/main/java/org/apache/impala/analysis/TableRef.java
File fe/src/main/java/org/apache/impala/analysis/TableRef.java:

http://gerrit.cloudera.org:8080/#/c/18829/5/fe/src/main/java/org/apache/impala/analysis/TableRef.java@176
PS5, Line 176: tableNumRowsHint_
> IMHO this can lead to inconsistency in the plan, depending on where the num
If user set different hint value for same table in sql, this will indeed lead 
to inconsistency in the plan, but this depend on user behavior, and this 
problem also exists for selectivity hint.

Each table get hint value in 'TableRef.java' separately. If we need to add this 
ensurence, we need to put all involved tables into a list, and then find same 
table, and check hint value. But this may not easy to implements, especially 
for multiple nesting sql. Do you have any suggestion?

Or we can just add some comments to explain that this hint is work on each 
single table, even we use different hint value for same table in a sql. 
Besides, what if the user just wants to do this? Maybe we can keep this setting 
for more flexibility.


http://gerrit.cloudera.org:8080/#/c/18829/6/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/18829/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@5046
PS6, Line 5046: Syntax error in line 1
> Yeah, I agree with you in that in this case it may be hard to figure out th
Thanks for testing this, it seem that we can ignore this problem now?


http://gerrit.cloudera.org:8080/#/c/18829/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@5048
PS6, Line 5048: Syntax error in line 1
> same comment as above.
Done


http://gerrit.cloudera.org:8080/#/c/18829/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@5051
PS6, Line 5051: "Syntax error in line 1
> same comment as above.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f0c773f4e67782a1428db64062f68afbd257af7
Gerrit-Change-Number: 18829
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Fucun Chu <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: wangsheng <[email protected]>
Gerrit-Comment-Date: Fri, 16 Sep 2022 02:45:09 +0000
Gerrit-HasComments: Yes

Reply via email to