Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7521/1/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
File fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java:

PS1, Line 185: if (bp.getChild(0).isImplicitCast()) return false;
> Will this also change the behaviour of implicitly casted integer types (e.g
The reason the bug doesn't affect integer types is because all of them are 
NumericLiterals, and so the evalBinaryPredicate() will be able to match the 
partition key literal with the predicate literal. The 'matching' in the FE uses 
java maps, so it works as long as the LiteralExprs are equal (i.e. java 
equal()).

With this change, BIGINT <op> INT should remain the same, but INT <op> BIGINT 
will fall back to the BE evaluation (it will still be partition pruned, just 
calls into the BE to evaluate the predicate). In the first case, the RHS INT 
gets promoted to a BIGINT, so no cast is necessary. In the latter case, the RHS 
can't be stored in the type of the LHS, so casting the LHS is necessary.

Given that the function evalBinaryPredicate(Expr) relies on the Literals to 
match, I think this change is the right thing to do. Keep in mind that if this 
returns false, it doesn't mean there won't be partition pruning, just that the 
partition pruning won't happen using the partition MD in the FE - it just has 
to call into the backend to evaluate the expr. I think that is acceptable.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Vincent Tran <[email protected]>
Gerrit-HasComments: Yes

Reply via email to