Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/15250 )
Change subject: IMPALA-6689: Speed up point lookup for Kudu primary key ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/15250/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: http://gerrit.cloudera.org:8080/#/c/15250/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@397 PS6, Line 397: if ((predicate instanceof BinaryPredicate) && > I think this will incorrectly include predicates that are comparing two col This new code is called only when function tryConvertBinaryKuduPredicate() return true. We already check those conditions in tryConvertBinaryKuduPredicate(). It's not necessary to check again. To avoid confusion, we can define the primaryKeyColInEquivalPred as member variable, and update its value in tryConvertBinaryKuduPredicate(). http://gerrit.cloudera.org:8080/#/c/15250/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@399 PS6, Line 399: SlotRef ref = (SlotRef) predicate.getChild(0); > We also need to check that the left child is a slotref. same reason as previous comment. -- To view, visit http://gerrit.cloudera.org:8080/15250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4631cd4d1a528a1152b5cdcb268426f2ba1a0c08 Gerrit-Change-Number: 15250 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Sat, 22 Feb 2020 04:45:17 +0000 Gerrit-HasComments: Yes
