Alex Behm has posted comments on this change. Change subject: IMPALA-2956: Filters should be able to target multiple scan nodes ......................................................................
Patch Set 1: (9 comments) More to come after we meet. http://gerrit.cloudera.org:8080/#/c/2932/1/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: Line 85: 3: required list<Exprs.TExpr> target_exprs There are several lists that store target-expr specific stuff. Would it make sense all the info into a new struct? For example, struct TFilterTargetExpr { Exprs.TExpr expr bool is_bound_by_partition_column list<Types.TSlotId> slotids bool is_local_target } http://gerrit.cloudera.org:8080/#/c/2932/1/fe/src/main/java/com/cloudera/impala/analysis/Analyzer.java File fe/src/main/java/com/cloudera/impala/analysis/Analyzer.java: Line 2441: LOG.info("@DT " + actual + "\n" + expected); remove? http://gerrit.cloudera.org:8080/#/c/2932/1/fe/src/main/java/com/cloudera/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/com/cloudera/impala/planner/RuntimeFilterGenerator.java: Line 115: // target scan nodes are in the same fragment as the broadcast join that produces not necessarily a broadcast join (could be num_nodes=1) Line 117: private final List<Boolean> isLocalTarget_ = Lists.newArrayList(); hasLocalTarget_? since this is a member of RuntimeFilter Line 518: // equivalent classes are computed, 'getTargetSlots()' may fail to identify target equivalence classes Line 523: for (RuntimeFilter filter: getRuntimeFilters()) { I'm not really a big fan of this approach, but you already know that :) It looks almost identical to computeTargetExpr() and does not address the root cause of why we need to do these special-case acrobatics for union. I'm more in favor of applying the logic here to all cases - but there will be some cost to pay. Let's do another brainstorming session with Marcel. http://gerrit.cloudera.org:8080/#/c/2932/1/testdata/workloads/functional-planner/queries/PlannerTest/hbase.test File testdata/workloads/functional-planner/queries/PlannerTest/hbase.test: Line 15: HBASE KEYRANGE port=16201 7:9 revert changes to keyranges http://gerrit.cloudera.org:8080/#/c/2932/1/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test File testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test: Line 674: union all add a test with UNION DISTINCT for completeness Line 741: select STRAIGHT_JOIN count(*) from functional.alltypes a nit: let's use consistent casing, e.g. all lowercase -- To view, visit http://gerrit.cloudera.org:8080/2932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad2ce4e579a30616c469312a4e658140d317507b Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-HasComments: Yes
