[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 7: Verified+1 -- 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: 7 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 30 Nov 2021 00:23:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18040 ) Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where keyword. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed original condition check and added a new check to ensure that all warnings for new/substituted predicates are already exists in 'globalState_.warnings'. And this will also avoiding exception caused by illegal hints. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Reviewed-on: http://gerrit.cloudera.org:8080/18040 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 51 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 8 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Zoltan Borok-Nagy 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 6: Code-Review+2 Looks great! Thanks for fixing 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: 6 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 18:02:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7678/ DRY_RUN=false -- 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: 7 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 18:03:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 7: Code-Review+2 -- 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: 7 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 18:03:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 6: Code-Review+1 -- 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: 6 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 03:33:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9845/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 6 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 02:15:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 6: (1 comment) Done! http://gerrit.cloudera.org:8080/#/c/18040/5/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/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3336 PS5, Line 3336: ' > nit: one space here instead of two Done -- 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: 6 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 01:55:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
wangsheng has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/18040 ) Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where keyword. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed original condition check and added a new check to ensure that all warnings for new/substituted predicates are already exists in 'globalState_.warnings'. And this will also avoiding exception caused by illegal hints. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 51 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/18040/6 -- 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: newpatchset Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 6 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/18040/5/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/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3336 PS5, Line 3336: nit: one space here instead of two -- 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 Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 29 Nov 2021 01:22:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9844/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Sun, 28 Nov 2021 03:36:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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
Gerrit-Reviewer: Amogh Margoor
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: wangsheng
Gerrit-Comment-Date: Sun, 28 Nov 2021 03:13:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
wangsheng has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/18040 ) Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where keyword. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed original condition check and added a new check to ensure that all warnings for new/substituted predicates are already exists in 'globalState_.warnings'. And this will also avoiding exception caused by illegal hints. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 51 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/18040/5 -- 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: newpatchset Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 5 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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
Gerrit-Reviewer: Amogh Margoor
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: wangsheng
Gerrit-Comment-Date: Sun, 28 Nov 2021 01:48:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9842/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 3 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Thu, 25 Nov 2021 17:18:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
wangsheng has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18040 ) Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where keyword. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed original condition check and added a new check to ensure that all warnings for new/substituted predicates are already exists in 'globalState_.warnings'. And this will also avoiding exception caused by illegal hints. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 62 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/18040/3 -- 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: newpatchset Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 3 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 2: Hi Quanlong, thanks for quick reply and explain. Here is some of my opinion. 1. The exception is threw when substituted expr execute analyze(), and called addWarning(). Before creating and initializing TExecRequest, any addWarning() called from exprs' analyze() will succeed. 2. Substituted expr is created by original expr's ctor through clone(), we cannot distinguish if this expr is new or substituted, unless we add a flag in Expr to indicate this, but I think this is too complex. 3. So we don't know if we should calling or avoiding addWarning() in this expr's analyze(), and this is why I just add a new check in addWarning(). This check ensure all substituted exprs' warnings already exists in 'warnings'. 4. Besides, addWarning() is not only called by Expr's analyze, and also other structs, such as StatementBase/TableDef and so on. How do you think? -- 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: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 24 Nov 2021 03:33:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18040/2/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/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3289
PS2, Line 3289: public void addWarning(String msg) {
Sorry that I didn't explain my meaning clearly. I haven't looked into all the
callers of this. But can we avoid calling 'addWarning' for the illegal cases? I
think this is what the precondition check expects.
BTW, I'm ok to the current solution if the above idea is too complex to
implement.
--
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: 2
Gerrit-Owner: wangsheng
Gerrit-Reviewer: Amogh Margoor
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: wangsheng
Gerrit-Comment-Date: Wed, 24 Nov 2021 01:54:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9836/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 23 Nov 2021 16:13:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 2: (3 comments) Hi Zoltan and Quanlong, thanks for your cr, I've already added a new check to replace the original check. How do you think? http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@10 PS1, Line 10: keyword. > nit: keyword Done http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@17 PS1, Line 17: New predicate : analyze will add hint warning to 'GlobalState.warnings' > Shouldn't we avoid adding warnings for new/substituted predicates which are Done http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@21 PS1, Line 21: This check is added in IMPALA-4166, I removed original condition check : and added a new check to ensure that all warnings for new/substituted : predicates are already exists in 'globalState_.warnings'. And this will : also avoiding exception caused by illegal hints. : > I think it would be nice to preserve something from the original intent, i. Done -- 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: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 23 Nov 2021 15:53:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
wangsheng has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18040 ) Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where keyword. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed original condition check and added a new check to ensure that all warnings for new/substituted predicates are already exists in 'globalState_.warnings'. And this will also avoiding exception caused by illegal hints. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 47 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/18040/2 -- 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: newpatchset Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
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 1: (1 comment) Thanks for fixing this bug! http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@17 PS1, Line 17: New predicate : analyze will add hint warning to 'GlobalState.warnings' Shouldn't we avoid adding warnings for new/substituted predicates which are not in the original query? -- 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: 1 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 23 Nov 2021 08:39:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Zoltan Borok-Nagy 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 1: (2 comments) Thanks for fixing this issue! http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@10 PS1, Line 10: key word nit: keyword http://gerrit.cloudera.org:8080/#/c/18040/1//COMMIT_MSG@21 PS1, Line 21: This check is added in IMPALA-4166, I removed this check and add new : test cases for predicate hints. This maybe lead to warnings added : but not displayed, I think this is ok, since warnings added later : is generated by substitute predicate, the original predicate is already : added displayed warnings when creating and initializing TExecRequest. I think it would be nice to preserve something from the original intent, i.e. at least make sure that there are no undisplayed warnings in the first phase. -- 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: 1 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 22 Nov 2021 16:32:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
Impala Public Jenkins 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 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9811/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 1 Gerrit-Owner: wangsheng Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 19 Nov 2021 07:24:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11021: Fix bug when query contains illegal predicate hints
wangsheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18040 Change subject: IMPALA-11021: Fix bug when query contains illegal predicate hints .. IMPALA-11021: Fix bug when query contains illegal predicate hints Currently Impala support predicate hint: ALWAYS_TRUE, we can use this hint after where key word. If we use illegal hints carelessly, query will throw IllegalStateException which is not expected. Query should return normal results with a warning instead of a exception. This is due to the condition check in Analyzer.addWarning(). After create TExecRequest and initialize it, Impala will get warnings from 'GlobalState.warnings', and 'GlobalState.warningsRetrieved' will be set to 'true' then. But after this, Impala will substitute predicate by clone(), and analyze new predicate in later phase. New predicate analyze will add hint warning to 'GlobalState.warnings', but failed and throw IllegalStateException due to 'globalState_.warningsRetrieved' check failed which is expected as 'false'. This check is added in IMPALA-4166, I removed this check and add new test cases for predicate hints. This maybe lead to warnings added but not displayed, I think this is ok, since warnings added later is generated by substitute predicate, the original predicate is already added displayed warnings when creating and initializing TExecRequest. Testing: - Added new fe tests in 'AnalyzeStmtsTest' Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 35 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/18040/1 -- 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: newchange Gerrit-Change-Id: Id719bc4280c811456333eb4b4ec5bc9cb8bae128 Gerrit-Change-Number: 18040 Gerrit-PatchSet: 1 Gerrit-Owner: wangsheng
