impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5180: Don't use non-deterministic exprs in partition pruning
Date Thu, 20 Apr 2017 19:16:55 GMT
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 <zamsden@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message