Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-2805: Order filters based on selectivity and cost
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/2598/1/fe/src/main/java/com/cloudera/impala/analysis/Expr.java
File fe/src/main/java/com/cloudera/impala/analysis/Expr.java:

Line 67:   public final static int BETWEEN_PREDICATE_COST = 1;
> Because we replace the BetweenPredicate's children with a CompoundPredicate
yes, between itself doesn't carry any extra cost.


Line 68:   public final static int BINARY_PREDICATE_COST = 1;
> The benefit of having these separate is that it allows us to express consta
agree, let's postpone. as part of a more principled calibration, based on a 
microbenchmark, we'll also want to switch to float instead of int.


Line 69:   public final static int CASE_COST = 5;
> The general sense that a case is more complicated, and therefore more costl
yes, this isn't necessary, it's simply the sum of its when clauses.


Line 80:   public final static int TIMESTAMP_ARITHMETIC_COST = 1;
> The advantage of grouping all of these costs here is that it makes it easie
works for me.


Line 184:   // its subexpressions.
> The reason I'm not doing this during analysis, and instead just computing c
yes, please move to analyze() and set explicitly in the c'tor of pre-analyzed 
nodes. having this live outside of analyze() is a further complication, because 
analyze is supposed to provide all semantic info (and again you can't guarantee 
that you'll have type information available, so you'll need to ensure that 
analyze already got called, which would introduce another control flow 
dependency)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02279a26fbc6308ac5eb819d78345fc010469034
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to