Alex Behm has posted comments on this change. Change subject: IMPALA-2805: Order conjuncts based on selectivity and cost ......................................................................
Patch Set 14: (5 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 Line 1256: if (ref.getResolvedPath() != null && ref.getResolvedPath().destColumn() != null && shorter way to get the stats ref.getDesc().getStats() 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 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 http://gerrit.cloudera.org:8080/#/c/2598/14/testdata/workloads/functional-planner/queries/PlannerTest/conjunct-ordering.test File testdata/workloads/functional-planner/queries/PlannerTest/conjunct-ordering.test: Line 1: # We don't estimate selectivity for 'slot_ref = slot_ref' Marcel, what is your take on the testing here? I think we should either expand the tests to cover conjuncts in more places, or we should leave them out and let the existing tests be the coverage. What do you think? -- 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
