Thomas Tauber-Marshall has posted comments on this change.

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


Patch Set 14:

(4 comments)

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

Line 1254:     SlotRef ref = e.unwrapSlotRef(false);
> CHAR types have a fixed size
Not sure what your point here is - it looks like getStats().getAvgSize() on a 
SlotRef representing a CHAR(N) will return N, which is what we want.


Line 1256:       if (ref.getResolvedPath() != null && 
ref.getResolvedPath().destColumn() != null &&
> shorter way to get the stats ref.getDesc().getStats()
Done


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

Line 69:   public void init(Analyzer analyzer) throws ImpalaException {
> exchange cannot have conjuncts
Done


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

Line 165:     conjuncts_ = orderConjunctsByCost(conjuncts_);
> this node has no conjuncts, see function comment
I did read the comment, but I guess I was confused by the 'if 
(!conjuncts_.isEmpty())' below in getNodeExplainString, whicn I guess can never 
be true.

Done.


-- 
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: 14
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Henry Robinson <[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