Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5602: Fix kudu queries being incorrectly optimized as small query ......................................................................
Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7560/2/fe/src/main/java/org/apache/impala/planner/PlanNode.java File fe/src/main/java/org/apache/impala/planner/PlanNode.java: PS2, Line 802: /** : * Returns true if this node does not contain any conjuncts. : */ : public boolean isConjunctsEmpty() { : // Can be overridden by inherited classes to consider all effective : // conjuncts like those pushed to kudu. : return getConjuncts().isEmpty(); : } This could be confusing because it's not obvious why someone would know to use this vs getConjuncts().isEmpty(). I think it'll be better to further differentiate by instead exposing a separate term in the interface, e.g. 'effective conjuncts' as we discussed. I think a clean interface will be public List<Expr> getEffectiveScanConjuncts() ... Which can be put in the ScanNode (there's no reason this needs to be on PlanNode). Yes, the code in MaxRowsProcessedVisitor has to then call .isEmpty(), but that's fine and I think exposing the conjuncts in this way makes sense. So to summarize: 1) move to ScanNode 2) change to getEffectiveScanConjuncts() -- To view, visit http://gerrit.cloudera.org:8080/7560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
