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

Change subject: IMPALA-11424: Support pushdown non-equi join predicate from 
OUTER/INNER JOIN to SCANNODE
......................................................................


Patch Set 4:

(43 comments)

http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
File fe/src/main/java/org/apache/impala/planner/HashJoinNode.java:

http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@139
PS4, Line 139:           && (this.getJoinOp().isLeftOuterJoin() || 
this.getJoinOp().isRightOuterJoin() || this.getJoinOp().isInnerJoin())) {
line too long (125 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@347
PS4, Line 347:    *
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@349
PS4, Line 349:    *  1. Get the mapping relationship between slot and non-equi 
conjunct list,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@351
PS4, Line 351:    *  2. For the case where there are equal and non-equi 
conjuncts in the slot at the same time,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@351
PS4, Line 351:    *  2. For the case where there are equal and non-equi 
conjuncts in the slot at the same time,
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@353
PS4, Line 353:    *  3. The maximum and minimum values are newly built into 
binaryPredicate according to non-equi conjunct;
line too long (108 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@371
PS4, Line 371:     if (this.joinOp_ == JoinOperator.LEFT_OUTER_JOIN || 
this.joinOp_ == JoinOperator.INNER_JOIN) {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@374
PS4, Line 374:     if (this.joinOp_ == JoinOperator.RIGHT_OUTER_JOIN || 
this.joinOp_ == JoinOperator.INNER_JOIN) {
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@379
PS4, Line 379:   private void pushdownNonEquiConjunct(Analyzer analyzer,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@380
PS4, Line 380:                 List<Expr> assignedConjunctsExprs, int 
buildIndex, int probeIndex) throws ImpalaException {
line too long (107 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@383
PS4, Line 383:             = 
groupAssignedConjunctsAccordingToSlotRef(assignedConjunctsExprs, buildIndex);
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@390
PS4, Line 390:     for (Map.Entry<SlotRef, List<Predicate>> entry : 
slotRefOtherJoinPredicateMap.entrySet()) {
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@414
PS4, Line 414:         
nonEquiConjunctList.addAll(pushOtherJoinBinaryPredicateToChild((BinaryPredicate)
 nonEquiPredicate,
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@421
PS4, Line 421:       //  KuduScanNode -> kuduPredicates_
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@437
PS4, Line 437:             List<Expr> conjunctList = 
conjunctTupleIdCorrect(analyzer, nonEquiConjunctList, 
scanNode.getTupleDesc().getId());
line too long (125 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@453
PS4, Line 453:   private List<Expr> conjunctTupleIdCorrect(Analyzer analyzer, 
List<Expr> conjuncts,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@480
PS4, Line 480:           BinaryPredicate newBinary = new 
BinaryPredicate(binaryPredicate.getOp(), expr, binaryPredicate.getChild(1));
line too long (118 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@491
PS4, Line 491:    * Determine whether BinaryPredicate can be pushed down, 
currently only supports the following:
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@495
PS4, Line 495:   private boolean canPushDownBinaryPredicate(BinaryPredicate 
binaryPredicate, SlotRef slotRef) {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@514
PS4, Line 514:    * Determine whether InPredicate can be pushed down, currently 
only supports the following:
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@533
PS4, Line 533:   private boolean canAssignedPredicatesPushDown(List<Predicate> 
assignedPredicates, SlotRef slotRef) {
line too long (102 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@539
PS4, Line 539:               && canPushDownBinaryPredicate((BinaryPredicate) 
assignedPredicate, slotRef)) {
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@567
PS4, Line 567:     // The key is build table's expr, the value is the 
corresponding Predicate collection containing LiteralExpr
line too long (112 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@577
PS4, Line 577:       if (!(predicate.getChild(0) instanceof LiteralExpr && 
!(predicate.getChild(0) instanceof NullLiteral))
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@577
PS4, Line 577:       if (!(predicate.getChild(0) instanceof LiteralExpr && 
!(predicate.getChild(0) instanceof NullLiteral))
line too long (109 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@578
PS4, Line 578:             && !(predicate.getChild(1) instanceof LiteralExpr && 
!(predicate.getChild(1) instanceof NullLiteral))) {
line too long (116 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@595
PS4, Line 595:             List<Predicate> predicateList = 
slotRefPredicateMap.getOrDefault(boundSlot, new ArrayList<>());
line too long (107 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@637
PS4, Line 637:             List<Predicate> predicateList = 
slotRefPredicateMap.getOrDefault(boundSlot, new ArrayList<>());
line too long (107 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@652
PS4, Line 652:    * Extracts the maximum value in the specified SlotRef from 
Predicates,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@655
PS4, Line 655:   private static LiteralExpr 
getMaxLiteralFromPredicates(List<Predicate> assignedPredicates, SlotRef 
slotRef) {
line too long (111 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@670
PS4, Line 670:         } else if (assignedPredicate.getChild(0).equals(slotRef) 
&& (op == LE || op == LT)) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@672
PS4, Line 672:         } else if (assignedPredicate.getChild(1).equals(slotRef) 
&& (op == GE || op == GT)) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@707
PS4, Line 707:    * Extracts the minimum value in the specified SlotRef from 
Predicates,
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@736
PS4, Line 736:         } else if (assignedPredicate.getChild(0).equals(slotRef) 
&& (op == GE || op == GT)) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@748
PS4, Line 748:         } else if (assignedPredicate.getChild(1).equals(slotRef) 
&& (op == LE || op == LT)) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@788
PS4, Line 788:     List<Expr> newConjuncts = new ArrayList<>();
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@832
PS4, Line 832:       // If operator is EQ, it means slotBinding = key, and
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@834
PS4, Line 834:       // so, predicate can be simplified to: slotBinding >= 
minvalue && slotBinding <= maxvalue.
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@848
PS4, Line 848:       // If operator is GT/GE, it means slotBinding > key or 
slotBinding >= key, and
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java@855
PS4, Line 855:       // If operator is LT/LE, it means slotBinding < key or 
slotBinding <= key, and
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:

http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2023
PS4, Line 2023:     boolean enableJoinPushdown = 
analyzer.getQueryOptions().isEnable_none_equal_predicate_push_down();
line too long (102 > 90)


http://gerrit.cloudera.org:8080/#/c/18731/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2029
PS4, Line 2029:     } else if (innerRef.getJoinOp().isSemiJoin() ||
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18731/4/tests/query_test/test_none_equi_predicate_pushdown.py
File tests/query_test/test_none_equi_predicate_pushdown.py:

http://gerrit.cloudera.org:8080/#/c/18731/4/tests/query_test/test_none_equi_predicate_pushdown.py@33
PS4, Line 33: c
flake8: E501 line too long (99 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3ce23cbd7522a209c830504f329b972d67bc263
Gerrit-Change-Number: 18731
Gerrit-PatchSet: 4
Gerrit-Owner: Baike Xia <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Mon, 18 Jul 2022 02:10:34 +0000
Gerrit-HasComments: Yes

Reply via email to