Alex Behm has posted comments on this change. Change subject: IMPALA-5180: Don't use non-deterministic exprs in partition pruning ......................................................................
Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/6575/3/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: Line 1029: public boolean isBoundByTupleIds(List<TupleId> tids) { > While consistency is nice, I'd rather keep this fix small and contained. I don't think consistency is just "nice" here. The isBound() family of functions is a core FE concept that we rely on in many places. It's bad if someone else reading the code cannot understand the meaning or gets confused due to inconsistency. These little inconsistencies have a tendency to cause other problems in the future. We are introducing this inconsistency for a rather minor bugfix, which I think is the wrong tradeoff. If the goal of this patch is to fix IMPALA-5180 specifically, then it's simpler to have HdfsPartitionPruner.prunePartitions() check if (conjunct.isBoundBySlotIds(partitionSlots_) && !conjunct.contains(Expr.IS_NONDETERMINISTIC_BUILTIN_FN_PREDICATE)) { // Use conjunct for partition pruning ... } and not change the meaning of isBoundBySlotIds() -- To view, visit http://gerrit.cloudera.org:8080/6575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I91054c6bf017401242259a1eff5e859085285546 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
