You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2020/04/25 01:08:38 UTC

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15807


Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................

IMPALA-9539: Enable CNF rewrites by default

This patch enables the conjunctive normal form rewrites
by default by setting enable_cnf_rewrites to true.
Functional and performance testing with this flag set to
true did not uncover any regressions and showed significant
performance gains for queries with disjunctions in the
tpch and tpcds suites.

Testing:
 - Updated the PlannerTest tests with plan changes
   in various test suites. Removed previously added tpch
   tests which were explicitly setting this flag to true.
 - I had previously added a test in convert-to-cnf.test
   with enable_cnf_rewrites=false, so I did not add any
   new tests with this flag disabled.

Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
---
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
8 files changed, 499 insertions(+), 660 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2: Code-Review+2

LGTM. Thanks for the work on CNF rewrites!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 02:00:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Apr 2020 02:37:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 01 May 2020 04:47:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 22:26:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15807/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java:

http://gerrit.cloudera.org:8080/#/c/15807/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@114
PS5, Line 114:           cpred = (CompoundPredicate) (cpred.clone());
Quanlong, the changes in this file are the only.additional change compared to what you had reviewed earlier.  I found that if  no transformation is done to the expression, we should return the original expr instead of the analyzed one because otherwise there were couple of test failures  related to constant propagation ( one in constant-propagation.test and one in hdfs.test).  I re-ran the gerrit-verify-dryrun to confirm all tests passed.  Could you pls review this additional change ?  Thanks.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:11:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:47:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 23:31:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 06:52:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................

IMPALA-9539: Enable CNF rewrites by default

This patch enables the conjunctive normal form rewrites
by default by setting enable_cnf_rewrites to true. Since
the CNF rule does an explicit analyze of the predicate
if it was not previously analyzed, in case no rewrite
was done we were previously returning the analyzed
predicate. This causes some side effects hence I have
fixed it by returning the original un-analyzed predicate
when no rewrite is done.

Other functional and performance testing with this flag
set to true did not uncover major regressions and showed
significant performance gains for queries with disjunctions
in the tpch and tpcds suites.

Testing:
 - Updated the PlannerTest tests with plan changes
   in various test suites. Removed previously added tpch
   tests which were explicitly setting this flag to true.
 - I had previously added a test in convert-to-cnf.test
   with enable_cnf_rewrites=false, so I did not add any
   new tests with this flag disabled.

Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
---
M common/thrift/ImpalaInternalService.thrift
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
9 files changed, 505 insertions(+), 662 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/15807/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 01:29:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Apr 2020 21:24:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15807/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java:

http://gerrit.cloudera.org:8080/#/c/15807/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@114
PS5, Line 114:           cpred = (CompoundPredicate) (cpred.clone());
> Quanlong, the changes in this file are the only.additional change compared 
Cool! This makes sense to me.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:46:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2:

> Patch Set 2:
> 
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5709/

The failed tests are related to OR predicate in an ORDER BY clause which is a separate issue. I have created IMPALA-9693 for it and posted a patch.  I tested the failed tests on my local machine (with code from both patches) and verified that those tests passed. Once that is merged, I will re-run gerrit-verify-dryrun here and confirm.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Apr 2020 15:33:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 21:43:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 07:26:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 02:01:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:47:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 02:57:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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/15807 )

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................

IMPALA-9539: Enable CNF rewrites by default

This patch enables the conjunctive normal form rewrites
by default by setting enable_cnf_rewrites to true. Since
the CNF rule does an explicit analyze of the predicate
if it was not previously analyzed, in case no rewrite
was done we were previously returning the analyzed
predicate. This causes some side effects hence I have
fixed it by returning the original un-analyzed predicate
when no rewrite is done.

Other functional and performance testing with this flag
set to true did not uncover major regressions and showed
significant performance gains for queries with disjunctions
in the tpch and tpcds suites.

Testing:
 - Updated the PlannerTest tests with plan changes
   in various test suites. Removed previously added tpch
   tests which were explicitly setting this flag to true.
 - I had previously added a test in convert-to-cnf.test
   with enable_cnf_rewrites=false, so I did not add any
   new tests with this flag disabled.

Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Reviewed-on: http://gerrit.cloudera.org:8080/15807
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/ImpalaInternalService.thrift
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
9 files changed, 505 insertions(+), 662 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 7
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9539: Enable CNF rewrites by default

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

Change subject: IMPALA-9539: Enable CNF rewrites by default
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dde86e092c61d71ddf9081f768072ced470b589
Gerrit-Change-Number: 15807
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Apr 2020 01:51:26 +0000
Gerrit-HasComments: No