Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18040 )

Change subject: IMPALA-11021: Fix bug when query contains illegal predicate 
hints
......................................................................


Patch Set 4:

(2 comments)

Thanks for updating the patch! The solution makes sense to me. Just asking for 
more clear comments.

http://gerrit.cloudera.org:8080/#/c/18040/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/18040/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3339
PS4, Line 3339: we removed original condition check
I think such kind of comments is better to be put in the commit message, rather 
than function comment here. We should explain what this method currently does, 
instead of how it changes.

I think we just need to mention: 'addWarning' may be called after the warnings 
are retrieved, e.g. in analyzing some substituted/cloned predicates 
(IMPALA-11021). We need to make sure no new warnings are added.


http://gerrit.cloudera.org:8080/#/c/18040/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3347
PS4, Line 3347:       int count = globalState_.warnings.get(msg);
              :       if (count == 1) {
              :         // Removed warning from map if logged only once time.
              :         globalState_.warnings.remove(msg);
              :       } else {
              :         // Just -1 if warning logged many times.
              :         globalState_.warnings.put(msg, count - 1);
              :       }
Could you explain the purpose for this? Do we really need this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128
Gerrit-Change-Number: 18040
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sky...@163.com>
Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.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: Sun, 28 Nov 2021 01:48:47 +0000
Gerrit-HasComments: Yes

Reply via email to