You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Qifan Chen (Code Review)" <ge...@cloudera.org> on 2022/04/28 20:31:09 UTC

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Qifan Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18458


Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11123: CNF Rewrite causes a regress in join node performance

This patch defines a common subset of all predicates that are common,
relatively inexpensive to compute, and can be converted to conjunctive
norm form (CNF) with low run-time overhead. Such predicates must involve
columns, constants or CAST only and are not one of the following.

  1. LIKE
  2. [NOT] EXISTS

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:
  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

Testing [TBD]

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
7 files changed, 53 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 1
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 8
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 30 Apr 2022 13:50:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 1
Gerrit-Owner: Qifan Chen <qc...@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, 28 Apr 2022 20:50:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 8:

(2 comments)

I'm working on updating this patch.

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG@32
PS3, Line 32:   2. Core tests;
> Definitely need to check that the transform is still applied in improved ca
https://github.com/apache/impala/blob/master/testdata/workloads/functional-planner/queries/PlannerTest/tpcds/tpcds-q13.test was originally added for that ticket and is unchanged by this patch.


http://gerrit.cloudera.org:8080/#/c/18458/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/18458/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS7, Line 605:   public void TestFeasibleToConvertToCNF() {
> I added two more tests on AND and OR but was not able to test it out as my 
TestFeasibleToConvertToCNF passes. I'd appreciate thoughts on examples for convert-to-cnf.test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 8
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 May 2022 17:46:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 3:

(4 comments)

Thanks for working on this! Is there a TPCH/TPCDS query that can reveal the regression?

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-11123
Wrong JIRA id?


http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG@32
PS3, Line 32: Testing [TBD]
We'd better make a run on perf-AB-test to see if this introduces other regressions: https://jenkins.impala.io/job/perf-AB-test/


http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java
File fe/src/main/java/org/apache/impala/analysis/Predicate.java:

http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java@90
PS3, Line 90: isColumnOrConstantPredicate
nit: maybe 'isSlotRefOrConstant' is better.


http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java@105
PS3, Line 105:     return isColumnOrConstantPredicate();
So we are rejecting all predicates that contain FunctionCallExpr. Does it make sense to add a whitelist for trivial functions, e.g. SIGN(), RADIANS(), DEGREES()?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@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, 29 Apr 2022 00:23:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive norm form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java;
  2. Core tests;
  3. perf-AB-test [TBD].

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
10 files changed, 129 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 7
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 14:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@64
PS14, Line 64:           "sign", "sin", "sinh", "sqrt", "tan", "tanh", "trunc", "truncate", "unhex"));
nit: this list could be stale if we add new builtin functions and forget to update it in the future. Maybe we should add some comments to mention this list in https://github.com/apache/impala/blob/882b6daf24a5be32706a2c1b8c069254d2f14033/common/function-registry/impala_functions.py#L325


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@830
PS14, Line 830:     return isBuiltinMathScalarFunction();
Should we check the children (if exist) as well? i.e. if isBuiltinMathScalarFunction() and !children_.isEmpty(), check all children recursively.


http://gerrit.cloudera.org:8080/#/c/18458/14/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/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS14, Line 605: TestShouldToConvertToCNF
nit: TestShouldConvertToCNF?


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@619
PS14, Line 619:         "select ((2 = cast(1 as int)) or (cast(0 as int) is not null))");
Can we add some coverage on those MathScalarFunctions?


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@623
PS14, Line 623:       assertTrue(expr.shouldConvertToCNF());
nit: It'd be nice to add the error message with the query.


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@630
PS14, Line 630:         "select (coalesce(d_date_sk, -1) = d_year) from tpcds_parquet.date_dim");
Can we add a case like log(length(upper(d_day_name)))? It covers the case of analyzing children of FunctionCallExpr.


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@634
PS14, Line 634:       assertFalse(expr.shouldConvertToCNF());
nit: It'd be nice to add the error message with the query.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 08:11:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#12) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in Predicates to avoid visiting the entire subtree on each
call to applyRuleBottomUp. Skips cache complexity on casts as they don't
branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
13 files changed, 251 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 17: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 17
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 May 2022 05:37:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 17: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 17
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 May 2022 01:08:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#10) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in Predicates to avoid visiting the entire subtree on each
call to applyRuleBottomUp. Skips cache complexity on casts as they don't
branch and are unlikely to be deeply nested.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java
  3. perf-AB-test

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
13 files changed, 204 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 10
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 11:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 11
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 May 2022 21:37:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/9/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/18458/9/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@101
PS9, Line 101:     if (!((CompoundPredicate)pred).feasibleToConvertToCNF()) {
This approach is a little painful because it's going to be O(n^2) where n is the number of AST nodes. Rules are evaluated bottom-up repeatedly until there are no more changes; each time that happens, it's going to walk each node's subtree to check whether it can be performantly converted to CNF.

I think the only way to improve that is cache the result on each predicate so we can look it up in later checks. However it seems difficult to reason about whether that cache would ever need to be invalidated, for example if there was a transform applied to an existing predicate. I've implemented the check on Predicate, but looking for feedback on whether it's safe.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 10
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 May 2022 21:12:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 2
Gerrit-Owner: Qifan Chen <qc...@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, 28 Apr 2022 20:58:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.

Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 7:

(3 comments)

Changes look good overall. A few comments below.

http://gerrit.cloudera.org:8080/#/c/18458/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/7//COMMIT_MSG@26
PS7, Line 26: norm
nit: normal


http://gerrit.cloudera.org:8080/#/c/18458/7/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/18458/7/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@102
PS7, Line 102:       LOG.info("It is not feasible to rewrite predidcate " + pred.toSql() + " to CNF.");
nit: at info level this may create lots of log entries since the rules are applied iteratively whenever any other transformation occurs.  Would be better to log at debug level.


http://gerrit.cloudera.org:8080/#/c/18458/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/18458/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS7, Line 605:   public void TestFeasibleToConvertToCNF() {
These tests don't have OR predicates.  One option is to add here but we should also add an actual PlannerTest with an Explain plan that has a string function that previously would have gotten converted through CNF and now is prohibited.  I can help create one test case if you don't get around to it.  It could be added in testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test (it used tpc-h schema).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 7
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Apr 2022 19:26:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 9
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 May 2022 19:19:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 11
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 17 May 2022 03:54:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@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, 28 Apr 2022 20:59:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 21:00:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 16:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 16
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 18:16:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 14:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@64
PS14, Line 64:           "sign", "sin", "sinh", "sqrt", "tan", "tanh", "trunc", "truncate", "unhex"));
> nit: this list could be stale if we add new builtin functions and forget to
I can do that. Is there any way to query the list of functions in that file directly?


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@830
PS14, Line 830:     return isBuiltinMathScalarFunction();
> Should we check the children (if exist) as well? i.e. if isBuiltinMathScala
Yeah. You could have a builtin call where the arguments include a non builtin function call.


http://gerrit.cloudera.org:8080/#/c/18458/14/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/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS14, Line 605: TestShouldToConvertToCNF
> nit: TestShouldConvertToCNF?
Ack


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@619
PS14, Line 619:         "select ((2 = cast(1 as int)) or (cast(0 as int) is not null))");
> Can we add some coverage on those MathScalarFunctions?
I'd added one (line 617), but I can add more.


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@623
PS14, Line 623:       assertTrue(expr.shouldConvertToCNF());
> nit: It'd be nice to add the error message with the query.
Ack


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@630
PS14, Line 630:         "select (coalesce(d_date_sk, -1) = d_year) from tpcds_parquet.date_dim");
> Can we add a case like log(length(upper(d_day_name)))? It covers the case o
Ack


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@634
PS14, Line 634:       assertFalse(expr.shouldConvertToCNF());
> nit: It'd be nice to add the error message with the query.
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 16:29:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

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

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in branching expressions to avoid visiting the entire
subtree on each call to applyRuleBottomUp. Skips cache complexity on
casts as they don't branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Reviewed-on: http://gerrit.cloudera.org:8080/18458
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
14 files changed, 279 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 18
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 13:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 13
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 20:51:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#11) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in Predicates to avoid visiting the entire subtree on each
call to applyRuleBottomUp. Skips cache complexity on casts as they don't
branch and are unlikely to be deeply nested.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java
  3. perf-AB-test

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
13 files changed, 204 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 11
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 11:

(1 comment)

Added handling of arithmetic expressions.

http://gerrit.cloudera.org:8080/#/c/18458/11/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/18458/11/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@224
PS11, Line 224: JOIN functional.alltypes b ON (CAST(2 AS BIGINT) + CAST(a.id AS BIGINT) =
This changed because it contains a non-constant arithmetic expression (e.g. a.int_col >= 0 + b.bigint_col). However a.int_col is a SlotRef, so maybe we should allow transformations that include arithmetic expressions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 11
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 18 May 2022 23:57:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 01:40:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive norm form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java;
  2. Core tests;
  3. perf-AB-test [TBD].

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
10 files changed, 139 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 6
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11123: CNF Rewrite causes a regress in join node performance

This patch defines a common subset of all predicates that are common,
relatively inexpensive to compute. Such predicates must involve
columns, constants or CAST only and are not one of the following.

  1. LIKE
  2. [NOT] EXISTS

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:
  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive norm form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing [TBD]

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
7 files changed, 53 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-11274
> Wrong JIRA id?
Done


http://gerrit.cloudera.org:8080/#/c/18458/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/7//COMMIT_MSG@26
PS7, Line 26: norm
> nit: normal
Done


http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java
File fe/src/main/java/org/apache/impala/analysis/Predicate.java:

http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java@90
PS3, Line 90:  i < children_.size(); ++i)
> nit: maybe 'isSlotRefOrConstant' is better.
Changed to isSlotRefOrConstantPredicate. 

Done.


http://gerrit.cloudera.org:8080/#/c/18458/3/fe/src/main/java/org/apache/impala/analysis/Predicate.java@105
PS3, Line 105: 
> So we are rejecting all predicates that contain FunctionCallExpr. Does it m
Good point. Done.


http://gerrit.cloudera.org:8080/#/c/18458/7/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/18458/7/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@102
PS7, Line 102:       LOG.debug("It is not feasible to rewrite predidcate " + pred.toSql() + " to CNF.");
> nit: at info level this may create lots of log entries since the rules are 
Done


http://gerrit.cloudera.org:8080/#/c/18458/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/18458/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS7, Line 605:   public void TestFeasibleToConvertToCNF() {
> These tests don't have OR predicates.  One option is to add here but we sho
I added two more tests on AND and OR but was not able to test it out as my mini-cluster stops working. Please run the test if you can: "mvn -f $IMPALA_HOME/fe/pom.xml test -e -Djava.compiler=NONE -ff -Dtest=ExprRewriterTest#TestFeasibleToConvertToCNF"

Thanks for the idea of adding a planner test. Sounds good to me.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 8
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 30 Apr 2022 13:32:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/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/18458/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS7, Line 605:   public void TestFeasibleToConvertToCNF() {
> I added 2 tests to convert-to-cnf.test. The first one which uses UPPER func
Thanks! I'll take a look at the 2nd. Still wrapping my head around the grammar, I'd like to do a bit more exploratory testing myself.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 9
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 May 2022 16:48:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 00:18:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/15/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/18458/15/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@632
PS15, Line 632:         "select (log10(cast(1 + length(upper(d_day_name)) as double)) > 1.0) from tpcds_parquet.date_dim");
line too long (107 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 15
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 17:46:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 21:09:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.

Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

Code changes look ok, but the word "feasible" isn't really appropriate in this case.
Feasible: possible to do easily or conveniently.

Consider using ShouldConvert or something along those lines instead.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 15:46:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 14:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 21 May 2022 00:11:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java;
  2. Core tests;
  3. perf-AB-test [TBD].

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
10 files changed, 125 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 8
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants or CAST only and are not one of the following.

  1. LIKE
  2. [NOT] EXISTS

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:
  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive norm form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java;
  2. Core tests;
  3. perf-AB-test [TBD].

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
10 files changed, 139 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 5
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 5
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Apr 2022 17:02:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

Ran into IMPALA-10316, restarted tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 21:10:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#13) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in Predicates to avoid visiting the entire subtree on each
call to applyRuleBottomUp. Skips cache complexity on casts as they don't
branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
13 files changed, 259 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 13
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.

Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 14: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 23 May 2022 12:57:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 16: Code-Review+2

LGTM. Carry Kurt's +1 to +2.

Thanks for taking over this patch and addresing all the comments, Michael!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 16
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 May 2022 01:05:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 11
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 May 2022 23:27:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 10:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 10
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 May 2022 21:28:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.

Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/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/18458/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS7, Line 605:   public void TestFeasibleToConvertToCNF() {
> TestFeasibleToConvertToCNF passes. I'd appreciate thoughts on examples for 
I added 2 tests to convert-to-cnf.test. The first one which uses UPPER function is expected to not be converted to CNF and it does not .. so this is the expected behavior.  However, the second query uses CAST function which I would expect to be converted to CNF. But it does not.  


# IMPALA-11274: Test with string functions in the disjunctive predicate.
# In this case the predicate is not converted to CNF
select count(*) from lineitem, orders
   where l_orderkey = o_orderkey and
    ((upper(l_returnflag) = 'Y' and upper(o_orderpriority) = 'HIGH')
      or (upper(l_returnflag) = 'N' and upper(o_orderpriority) = 'LOW'))
and l_partkey > 0;

# IMPALA-11274: Functions like CAST should still be eligible for CNF
select count(*) from lineitem, orders
   where l_orderkey = o_orderkey and
    ((cast(l_returnflag as varchar(2)) = 'Y' and cast(o_orderpriority as varchar(5)) = 'HIGH')
      or (cast(l_returnflag as varchar(2)) = 'N' and cast(o_orderpriority as varchar(5)) = 'LOW'))
and l_partkey > 0;



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 9
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 14 May 2022 02:13:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.

Qifan Chen has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11123: CNF Rewrite causes a regress in join node performance

This patch defines a common subset of all predicates that are common,
relatively inexpensive to compute.  Such predicates must involve
columns, constants or CAST only and are not one of the following.

  1. LIKE
  2. [NOT] EXISTS

Examples of the subset of predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:
  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be convetted to
conjunctive norm form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead with CNFs in which some
of the predicates can be duplicated.

Testing [TBD]

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
7 files changed, 53 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 2
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11123: CNF Rewrite causes a regress in join node performance

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.

Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11123: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18458/3//COMMIT_MSG@32
PS3, Line 32: Testing [TBD]
> We'd better make a run on perf-AB-test to see if this introduces other regr
Definitely need to check that the transform is still applied in improved cases mentioned in IMPALA-9183.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Apr 2022 15:35:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

> Patch Set 12:
> 
> Code changes look ok, but the word "feasible" isn't really appropriate in this case.
> Feasible: possible to do easily or conveniently.
> 
> Consider using ShouldConvert or something along those lines instead.

Updated the function name and cleaned up comments a bit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 20:32:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#14) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in Predicates to avoid visiting the entire subtree on each
call to applyRuleBottomUp. Skips cache complexity on casts as they don't
branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
13 files changed, 259 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#15) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in branching expressions to avoid visiting the entire
subtree on each call to applyRuleBottomUp. Skips cache complexity on
casts as they don't branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
14 files changed, 278 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/15
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 15
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#16) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Uses a cache in branching expressions to avoid visiting the entire
subtree on each call to applyRuleBottomUp. Skips cache complexity on
casts as they don't branch and are unlikely to be deeply nested.

Testing:
- New expression writer tests
- New planner tests

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
14 files changed, 279 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/16
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 16
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 15:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 15
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 17:42:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 6
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Apr 2022 17:08:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 7
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Apr 2022 17:23:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18458/9/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/18458/9/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@101
PS9, Line 101:     if (!((CompoundPredicate)pred).feasibleToConvertToCNF()) {
> Storing the flag on the predicate should be fine. The input expressions sho
Done


http://gerrit.cloudera.org:8080/#/c/18458/11/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/18458/11/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@224
PS11, Line 224: JOIN functional.alltypes b ON CAST(2 AS BIGINT) + CAST(a.id AS BIGINT) =
> This changed because it contains a non-constant arithmetic expression (e.g.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 16:30:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 12:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 16:31:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has uploaded a new patch set (#9) to the change originally created by Qifan Chen. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................

IMPALA-11274: CNF Rewrite causes a regress in join node performance

This patch defines a subset of all predicates that are common and
relatively inexpensive to compute. Such predicates must involve
columns, constants, simple math or cast functions only.

Examples of the subset of the predicates allowed:

  1. (a = 1 AND cast(b as int) = 2) OR (c = d AND e = f)
  2. a in ('1', '2', '3') OR ((b = 'abc') AND (c = d))
  3. (a between 1 and 100) OR ((b is null) AND (c = d))

Examples of the predicates not allowed:

  1. (upper(a) != 'Y') AND b = 2) OR (c = d AND e = f)
  2. (coalesce(CAST(a AS string), '') = '') AND b = 2) OR
     (c = d AND e = f)

This patch further restricts the predicates to be converted to
conjunctive normal form (CNF) to be such a subset, with the aim to
reduce the run-time evaluation overhead of CNFs in which some
of the predicates can be duplicated.

Testing:
  1. Added a new test TestFeasibleToConvertToCNF() in ExprRewriterTest.java
  2. Ran fe tests
  3. perf-AB-test [TBD]

Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
---
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
11 files changed, 124 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/18458/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 9
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.

Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/9/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/18458/9/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@101
PS9, Line 101:     if (!((CompoundPredicate)pred).feasibleToConvertToCNF()) {
> This approach is a little painful because it's going to be O(n^2) where n i
Storing the flag on the predicate should be fine. The input expressions should generally be immutable and worst cast the transform would just get skipped. You can just verify some simple cases.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 9
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 17 May 2022 14:20:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.

Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/13/testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
File testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test:

http://gerrit.cloudera.org:8080/#/c/18458/13/testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test@439
PS13, Line 439: # IMPALA-11274: Simple arithmetic expressions should still be eligible for CNF
This predicate is already in CNF form so the rule would be a no-op.  Was there a specific reason to add this test ? If the intent is to test. CNF for predicates with arithmetic exprs, you can add OR predicates along the patterns of preceding examples and substitute one or more exprs with arithmetic exprs.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 13
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 23:18:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18458/13/testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test
File testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test:

http://gerrit.cloudera.org:8080/#/c/18458/13/testdata/workloads/functional-planner/queries/PlannerTest/convert-to-cnf.test@439
PS13, Line 439: # IMPALA-11274: Simple arithmetic expressions should still be eligible for CNF
> This predicate is already in CNF form so the rule would be a no-op.  Was th
Yeah, made it too simple. Added something that is transformed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 14
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 23:52:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 15:

(7 comments)

Addressed comments.

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@64
PS14, Line 64:       new HashSet<String>(Arrays.asList("abs", "acos", "asin", "atan", "atan2", "bin",
> I can do that. Is there any way to query the list of functions in that file
Seems like this will need to be a judgement call for the moment. No property on the function object that states it's a "math operation on scalar values". We could look for all functions that only operate on scalar arguments or no arguments, but that might have side-effects like including uuid/sleep.


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@830
PS14, Line 830:         if (!getChild(i).shouldConvertToCNF()) return false;
> Yeah. You could have a builtin call where the arguments include a non built
Done


http://gerrit.cloudera.org:8080/#/c/18458/14/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/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@605
PS14, Line 605: TestShouldConvertToCNF()
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@619
PS14, Line 619:         "select ((2 = cast(1 as int)) or (cast(0 as int) is not null))",
> I'd added one (line 617), but I can add more.
Done


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@623
PS14, Line 623:       Expr expr = analyze(query);
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@630
PS14, Line 630:         "select (d_day_name like '%A') from tpcds_parquet.date_dim",
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/18458/14/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@634
PS14, Line 634:     for (String query: inconvertablePredicates) {
> Ack
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 15
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 17:19:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11274: CNF Rewrite causes a regress in join node performance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18458 )

Change subject: IMPALA-11274: CNF Rewrite causes a regress in join node performance
......................................................................


Patch Set 17:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326406c6b004fe31ec0e2a2f390a3845b8925aa9
Gerrit-Change-Number: 18458
Gerrit-PatchSet: 17
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 May 2022 01:08:27 +0000
Gerrit-HasComments: No