wangsheng 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 5: (2 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: > I think such kind of comments is better to be put in the commit message, ra Done http://gerrit.cloudera.org:8080/#/c/18040/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3347 PS4, Line 3347: } : : /** : * Registers a new PrivilegeRequest in the analyzer. : */ : public void registerPrivReq(PrivilegeRequest privReq) { : if (!enablePrivChecks_) return; : if > Could you explain the purpose for this? Do we really need this? My original idea is to make sure that all substituted/cloned predicates' warnings not only exists in map, but also number is right. But I suddenly think of that maybe not all predicates would generated corresponding substituted predicate, so this check is not right. We just need to ensure that substituted predicates' warning already exists in map. So I removed this check. -- 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: 5 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 03:13:20 +0000 Gerrit-HasComments: Yes
