Marcel Kornacker has posted comments on this change.

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


Patch Set 2:

(8 comments)

please revise your use of blank lines; see in-line comments for more details

http://gerrit.cloudera.org:8080/#/c/2598/2/fe/src/main/java/com/cloudera/impala/planner/EmptySetNode.java
File fe/src/main/java/com/cloudera/impala/planner/EmptySetNode.java:

Line 56:     conjuncts_ = orderConjuncts(conjuncts_);
remove; this has no effect (this node always produces an empty set)


http://gerrit.cloudera.org:8080/#/c/2598/2/fe/src/main/java/com/cloudera/impala/planner/HBaseScanNode.java
File fe/src/main/java/com/cloudera/impala/planner/HBaseScanNode.java:

Line 131: 
avoid single-statement groups/double-spacing.


http://gerrit.cloudera.org:8080/#/c/2598/2/fe/src/main/java/com/cloudera/impala/planner/HashJoinNode.java
File fe/src/main/java/com/cloudera/impala/planner/HashJoinNode.java:

Line 100: 
remove blank line or move; these 3 lines belong together.


http://gerrit.cloudera.org:8080/#/c/2598/2/fe/src/main/java/com/cloudera/impala/planner/PlanNode.java
File fe/src/main/java/com/cloudera/impala/planner/PlanNode.java:

Line 639:   public static <T extends Expr> List<T> orderConjuncts(List<T> 
conjuncts) {
let's stick that in Expr.java


Line 643:     List<T> hasSelectivity = new ArrayList<T>();
withSelectivity?


Line 659:       T smallestConjunct =  null;
optConjunct?


Line 679:         @Override
remove override


Line 684: 
remove gratuitous blank lines, like this one


-- 
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: 2
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