You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zihao Ye (Code Review)" <ge...@cloudera.org> on 2024/07/09 03:40:45 UTC

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Zihao Ye has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21568


Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................

IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Currently, ExprRewriter cannot rewrite 'id = 0 OR false' to 'id = 0' as
expected. More precisely, it fails to rewrite any cases where a boolean
literal follows 'AND/OR' as expected.
The issue is that the CompoundPredicate generated by NormalizeExprsRule
is not analyzed, causing SimplifyConditionalsRule to skip the rewrite.
This patch fixes the issue by adding analysis of the rewritten
CompoundPredicate in NormalizeExprsRule.

Testing:
- Modified and passed FE test case
  ExprRewriteRulesTest#testCompoundPredicate

Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
---
M fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
2 files changed, 14 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/21568/1
-- 
To view, visit http://gerrit.cloudera.org:8080/21568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 1:

Thanks for the review!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 12:34:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16469/ : 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/21568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 04:06:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................

IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Currently, ExprRewriter cannot rewrite 'id = 0 OR false' to 'id = 0' as
expected. More precisely, it fails to rewrite any cases where a boolean
literal follows 'AND/OR' as expected.
The issue is that the CompoundPredicate generated by NormalizeExprsRule
is not analyzed, causing SimplifyConditionalsRule to skip the rewrite.
This patch fixes the issue by adding analysis of the rewritten
CompoundPredicate in NormalizeExprsRule.

Testing:
- Modified and passed FE test case
  ExprRewriteRulesTest#testCompoundPredicate
- Modified and passed related test case

Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Reviewed-on: http://gerrit.cloudera.org:8080/21568
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/empty.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
6 files changed, 23 insertions(+), 20 deletions(-)

Approvals:
  Csaba Ringhofer: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 1: Code-Review+2

looks great, thanks for fixing this!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 09:19:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10778/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 17:39:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 12:35:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16475/ : 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/21568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 10 Jul 2024 08:08:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10780/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 10 Jul 2024 07:45:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10778/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 12:35:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 10 Jul 2024 11:16:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21568 )

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 10 Jul 2024 12:53:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21568

to look at the new patch set (#3).

Change subject: IMPALA-13203: Rewrite 'id = 0 OR false' as expected
......................................................................

IMPALA-13203: Rewrite 'id = 0 OR false' as expected

Currently, ExprRewriter cannot rewrite 'id = 0 OR false' to 'id = 0' as
expected. More precisely, it fails to rewrite any cases where a boolean
literal follows 'AND/OR' as expected.
The issue is that the CompoundPredicate generated by NormalizeExprsRule
is not analyzed, causing SimplifyConditionalsRule to skip the rewrite.
This patch fixes the issue by adding analysis of the rewritten
CompoundPredicate in NormalizeExprsRule.

Testing:
- Modified and passed FE test case
  ExprRewriteRulesTest#testCompoundPredicate
- Modified and passed related test case

Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
---
M fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/empty.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
6 files changed, 23 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/21568/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d9fffdd1cc644cc2b48f08c2509f22a72362d22
Gerrit-Change-Number: 21568
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>