You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Abhishek Rawat (Code Review)" <ge...@cloudera.org> on 2022/06/02 01:39:37 UTC
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Abhishek Rawat has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18581
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
IMPALA-11284: Enable expression rewrites for || and Between predicate
in values clause
IMPALA-6590 disabled expression rewrites for ValuesStmt.
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with
boolean arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enabled expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or CompoundVerticalBarExpr.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 51 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18581/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18581/2//COMMIT_MSG@20
PS2, Line 20: CompoundVerticalBarExpr
nit: BetweenPredicate
http://gerrit.cloudera.org:8080/#/c/18581/2/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/2/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS2, Line 149:
nit: extra space
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 22:13:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13598/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 19:30:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#9) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
5 files changed, 77 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/9
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@711
PS7, Line 711: requireMandatoryRewrite_ = resultExpr.contains(
> Why is this here with the value transfer logic instead of in Analyze?
Not sure I understand the question.
This is done at SetOperationStmt.createMetadata() method, which is called by SetOperationStmt.analyze() method.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Sep 2023 23:34:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@186
PS7, Line 186: is
Nit: are.
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
File fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java@95
PS7, Line 95: into
Nit: to?
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@152
PS7, Line 152: // Values stmt - expression rewrites are disabled.
This is no longer completely true - we should update the comment to say something like expression rewrites are enabled except for || and the Between predicate, or that most expression rewrites are disabled.
http://gerrit.cloudera.org:8080/#/c/18581/7/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/7/testdata/workloads/functional-query/queries/QueryTest/values.test@148
PS7, Line 148: create table values_11284(col string, value boolean);
You don't need to create a table, you could run a select query like this:
select * from
(
values (concat("a", "b" || "c"), 1 <= 2 AND ((0.5 BETWEEN 0 AND 1) AND (true || false))),
("hello" || "world", 0 <= 1 || 0.5 < 0.6),
("impala", 4.0 BETWEEN 3.2 AND 4.1),
("sql", 'a' NOT BETWEEN 'b' AND 'c')
) t;
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Sep 2023 13:29:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11: Verified+1
> Patch Set 11: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9711/
All tests pass except 1 known flaky test tracked at IMPALA-12266.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Sep 2023 20:54:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@439
PS9, Line 439: private static boolean isRequireMandatoryRewrite(List<SetOperand> operands) {
> Looking again, I think requireMandatoryRewrite_ should not be reset like it
Thanks, it makes sense.
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@92
PS9, Line 92: if (!requireMandatoryRewrite_) return;
What if we didn't have the 'requireMandatoryRewrite_' field and instead just had the below ExprRewriter in all cases? It would still only rewrite the expressions that need to be rewritten.
It would make the code simpler, though maybe a bit less performant (the expr tree would have to be traversed for both rules instead of checking once whether any || or BETWEEN expr occurs in it). What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 09:27:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#5) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
6 files changed, 94 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/5
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 4:
ps4 is a rebase.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 16:48:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10681/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 02:00:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@92
PS9, Line 92: // IMPALA-11284: Don't skip rewrites for || and BETWEEN operator as the backend
> I think that should be fine too. BetweenToCompoundRule and ExtractCompoundV
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 16:06:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has removed a vote on this change.
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Removed Code-Review+2 by Aman Sinha <am...@cloudera.com>
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
IMPALA-11284: Enable expression rewrites for || and Between predicate
in values clause
IMPALA-6590 disabled expression rewrites for ValuesStmt.
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with
boolean arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enabled expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or CompoundVerticalBarExpr.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 50 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/2
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@711
PS7, Line 711: requireMandatoryRewrite_ = resultExpr.contains(
Why is this here with the value transfer logic instead of in Analyze?
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Sep 2023 23:18:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13605/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Jul 2023 00:45:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 05:34:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
IMPALA-11284: Enable expression rewrites for || and Between predicate
in values clause
IMPALA-6590 disabled expression rewrites for ValuesStmt.
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with
boolean arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 50 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/3
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#6) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
6 files changed, 97 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/6
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 10: Code-Review+1
(1 comment)
Thanks Riza.
http://gerrit.cloudera.org:8080/#/c/18581/10/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/10/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@91
PS10, Line 91: public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException {
Since we've removed the function doc comment about leaving the function empty, the below comment could be extended saying that only non-optional rewrites are done.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 16:36:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#7) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
5 files changed, 62 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/7
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 03:29:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#10) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 51 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/10
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@439
PS9, Line 439: private static boolean isRequireMandatoryRewrite(List<SetOperand> operands) {
> In the version in PS7, 'equireMandatoryRewrite_' could be reset to false if
Looking again, I think requireMandatoryRewrite_ should not be reset like it did in PS7.
Assignment in PS9 is the right one. It will enable mandatory rewrites for all operands even if only one of them has CompoundVerticalBarExpr or BetweenPredicate. The rewrites will become a no-op for operand that does not require it.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Sep 2023 15:02:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 8:
Patch set 8 is a rebase. I will address the comments in the next patch set.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Sep 2023 18:46:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@186
PS7, Line 186: op
> Nit: are.
Done
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@711
PS7, Line 711: // Add to aliasSMap so that column refs in "order by" can be resolved.
> This function does things related to tuple materialization. it's not clear
Moved this to analyzeOperands()
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
File fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java@95
PS7, Line 95: to t
> Nit: to?
Done
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@152
PS7, Line 152: // Values stmt - expression rewrites are not required in this test cases.
> This is no longer completely true - we should update the comment to say som
Done
http://gerrit.cloudera.org:8080/#/c/18581/7/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/7/testdata/workloads/functional-query/queries/QueryTest/values.test@148
PS7, Line 148: (
> You don't need to create a table, you could run a select query like this:
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Sep 2023 20:21:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#4) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
IMPALA-11284: Enable expression rewrites for || and Between predicate
in values clause
IMPALA-6590 disabled expression rewrites for ValuesStmt.
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with
boolean arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 49 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/4
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9711/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Sep 2023 16:09:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/10/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/10/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@91
PS10, Line 91: public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException {
> Since we've removed the function doc comment about leaving the function emp
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 16:56:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Sep 2023 21:38:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9:
(1 comment)
Thanks Riza. I just have a question, I can +2 after that if nobody else wants to review it.
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@439
PS9, Line 439: private static boolean isRequireMandatoryRewrite(List<SetOperand> operands) {
In the version in PS7, 'equireMandatoryRewrite_' could be reset to false if the 'resultExpr' of some operand didn't contain || or BETWEEN. In this version it can't be reset. Was it a bug in the older version?
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Sep 2023 11:10:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13958/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Sep 2023 20:45:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10718/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 02:00:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/5/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:
http://gerrit.cloudera.org:8080/#/c/18581/5/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@164
PS5, Line 164: "values(1 <= 2 AND ((0.5 BETWEEN 0 AND 1) AND (('a' || 'b') = 'ab' AND (true || false))))",
line too long (99 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 19:08:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13990/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 17:23:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has removed a vote on this change.
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has uploaded a new patch set (#11) to the change originally created by Abhishek Rawat. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
---
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 54 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/18581/11
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/6/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/6/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@92
PS6, Line 92: ExprRewriter nonOptionalRewriter = rewriter.copyNonOptionalRewriter();
This can probably be simplified more by creating new ExprRewriter that only consist of BetweenToCompoundRule and ExtractCompoundVerticalBarExprRule.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 22:53:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10687/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 17:38:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3: Code-Review+2
Carry both +1s and bumping to +2.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 05:34:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18581/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18581/2//COMMIT_MSG@20
PS2, Line 20: CompoundVerticalBarExpr
> nit: BetweenPredicate
Done
http://gerrit.cloudera.org:8080/#/c/18581/2/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/2/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS2, Line 149:
> nit: extra space
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 01:41:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18581/3/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/3/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@95
PS3, Line 95:
nit: extra line
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS3, Line 149: "hello" || "world", 0 <= 1 || 0.5 < 0.6
Can you also add a test with a || and between are not the root expressions, but are deeper in the expression tree?
For example concat("a", "b" || "c")
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 12:01:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 09 Sep 2023 00:45:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9711/
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Sep 2023 20:47:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/6/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/6/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@92
PS6, Line 92: (!requireMandatoryRewrite_) return;
> This can probably be simplified more by creating new ExprRewriter that only
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Jul 2023 00:24:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13596/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 17:08:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS3, Line 149: concat("a", "b" || "c"), 1 <= 2 AND ((0
> Looked into this a bit and realized that Impala already distinguishes betwe
Done
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 19:09:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/1/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/1/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@672
PS1, Line 672: slotDesc.addSourceExpr(resultExpr);
tab used for whitespace
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 01:40:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS3, Line 149: "hello" || "world", 0 <= 1 || 0.5 < 0.6
> Yeah looks like we are not handling this if the said operators are deeper i
Looked into this a bit and realized that Impala already distinguishes between "optional" and "non-optional" rewrite rules: https://github.com/apache/impala/blob/23d09638de35dcec6419a5e30df08fd5d8b27e7d/fe/src/main/java/org/apache/impala/analysis/Analyzer.java#L556
So ExtractCompoundVerticalBarExprRule is always done, even if enable_expr_rewrites false. I think that what we actually want here is to run all "non-optional" rules in the VALUES close, but skip "optional" ones, as in case a new "non-optional" rule will be added, VALUES will need to apply them automatically.
ExprRewriter could be extended to get an "non-optional" and an "optional" rule list. The rewrite() function would apply the concatenated list of "non-optional" and "optional" rules, while a rewriteNonOptional() could be added that would be called from VALUES.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jun 2022 09:46:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Enable expression rewrites for || and Between predicate in values clause
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18581/3/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/3/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@95
PS3, Line 95:
> nit: extra line
Done
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/18581/3/testdata/workloads/functional-query/queries/QueryTest/values.test@149
PS3, Line 149: "hello" || "world", 0 <= 1 || 0.5 < 0.6
> Can you also add a test with a || and between are not the root expressions,
Yeah looks like we are not handling this if the said operators are deeper in the expression tree. I will upload a new patch with the fix. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 19:11:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13599/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jul 2023 19:37:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
IMPALA-11284: Do non-optional rewrites for || and Between predicate
IMPALA-6590 disabled expression rewrites for ValuesStmt. However,
CompoundVerticalBarExpr (||) cannot be executed directly without
rewrite. This is because it could either be an OR operation with boolean
arguments or CONCAT function call with string arguments.
Backend cannot evaluate a BetweenPredicate and relies on rewriting
BetweenPredicate into a conjunctive or disjunctive CompoundPredicate.
This patch enables non-optional expression rewrites for ValuesStmt with
CompoundVerticalBarExpr or BetweenPredicate.
Testing:
- Extended ExprRewriterTest and Planner test to have values clause
with || and Between predicate
Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Reviewed-on: http://gerrit.cloudera.org:8080/18581
Reviewed-by: Michael Smith <mi...@cloudera.com>
Reviewed-by: Daniel Becker <da...@cloudera.com>
Tested-by: Riza Suminto <ri...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/values.test
4 files changed, 54 insertions(+), 7 deletions(-)
Approvals:
Michael Smith: Looks good to me, but someone else must approve
Daniel Becker: Looks good to me, approved
Riza Suminto: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Sep 2023 16:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java
File fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/7/fe/src/main/java/org/apache/impala/analysis/SetOperationStmt.java@711
PS7, Line 711: requireMandatoryRewrite_ = resultExpr.contains(
> Not sure I understand the question.
This function does things related to tuple materialization. it's not clear when the new flag gets used. I would think the transform should be completed before any materialization logic.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Sep 2023 15:04:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java:
http://gerrit.cloudera.org:8080/#/c/18581/9/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@92
PS9, Line 92: if (!requireMandatoryRewrite_) return;
> What if we didn't have the 'requireMandatoryRewrite_' field and instead jus
I think that should be fine too. BetweenToCompoundRule and ExtractCompoundVerticalBarExprRule will return early if the predicate they're aiming does not exist in the operand. I will make this change in the next patch set.
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 15:28:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13987/ : 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/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 16:31:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11284: Do non-optional rewrites for || and Between predicate
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18581 )
Change subject: IMPALA-11284: Do non-optional rewrites for || and Between predicate
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b8b33bf6468d12b9e26f0a6e744feb7072619c
Gerrit-Change-Number: 18581
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Sep 2023 18:44:40 +0000
Gerrit-HasComments: No