[teiid-commits] teiid SVN: r4457 - in trunk/engine/src: test/java/org/teiid/query/processor and 1 other directory.
teiid-commits at lists.jboss.org
teiid-commits at lists.jboss.org
Thu Sep 20 15:43:05 EDT 2012
Author: shawkins
Date: 2012-09-20 15:43:05 -0400 (Thu, 20 Sep 2012)
New Revision: 4457
Modified:
trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushAggregates.java
trunk/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java
Log:
TEIID-2220 fix for bad aggregate pushing with more complicated grouping and equi-join expressions.
Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushAggregates.java
===================================================================
--- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushAggregates.java 2012-09-18 19:18:44 UTC (rev 4456)
+++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushAggregates.java 2012-09-20 19:43:05 UTC (rev 4457)
@@ -645,11 +645,12 @@
Set<Expression> stagedGroupingSymbols = new LinkedHashSet<Expression>();
Collection<AggregateSymbol> aggregates = aggregateMap.get(planNode);
- if (!canPush(groupNode, stagedGroupingSymbols, planNode)) {
- continue;
+ planNode = canPush(groupNode, stagedGroupingSymbols, planNode);
+ if (planNode == null) {
+ continue;
}
- filterJoinColumns(stagedGroupingSymbols, planNode.getGroups(), groupingExpressions);
+ filterExpressions(stagedGroupingSymbols, planNode.getGroups(), groupingExpressions, false);
collectSymbolsFromOtherAggregates(allAggregates, aggregates, planNode, stagedGroupingSymbols);
@@ -776,45 +777,70 @@
/**
* Ensures that we are only pushing through inner equi joins or cross joins. Also collects the necessary staged grouping symbols
+ * @return null if we cannot push otherwise the target join node
*/
- private boolean canPush(PlanNode groupNode,
+ private PlanNode canPush(PlanNode groupNode,
Set<Expression> stagedGroupingSymbols,
PlanNode planNode) {
PlanNode parentJoin = planNode.getParent();
Set<GroupSymbol> groups = FrameUtil.findJoinSourceNode(planNode).getGroups();
+ PlanNode result = planNode;
while (parentJoin != groupNode) {
if (parentJoin.getType() != NodeConstants.Types.JOIN
|| parentJoin.hasCollectionProperty(NodeConstants.Info.NON_EQUI_JOIN_CRITERIA)
|| ((JoinType)parentJoin.getProperty(NodeConstants.Info.JOIN_TYPE)).isOuter()) {
- return false;
+ return null;
}
-
+ //we move the target up if the filtered expressions introduce outside groups
if (planNode == parentJoin.getFirstChild()) {
- if (parentJoin.hasCollectionProperty(NodeConstants.Info.LEFT_EXPRESSIONS)) {
- filterJoinColumns(stagedGroupingSymbols, groups, (List<Expression>)parentJoin.getProperty(NodeConstants.Info.LEFT_EXPRESSIONS));
+ if (parentJoin.hasCollectionProperty(NodeConstants.Info.LEFT_EXPRESSIONS)
+ && filterExpressions(stagedGroupingSymbols, groups, (List<Expression>)parentJoin.getProperty(NodeConstants.Info.LEFT_EXPRESSIONS), true)) {
+ result = parentJoin;
+ groups = result.getGroups();
}
} else {
- if (parentJoin.hasCollectionProperty(NodeConstants.Info.RIGHT_EXPRESSIONS)) {
- filterJoinColumns(stagedGroupingSymbols, groups, (List<Expression>)parentJoin.getProperty(NodeConstants.Info.RIGHT_EXPRESSIONS));
+ if (parentJoin.hasCollectionProperty(NodeConstants.Info.RIGHT_EXPRESSIONS)
+ && filterExpressions(stagedGroupingSymbols, groups, (List<Expression>)parentJoin.getProperty(NodeConstants.Info.RIGHT_EXPRESSIONS), true)) {
+ result = parentJoin;
+ groups = result.getGroups();
}
}
planNode = parentJoin;
parentJoin = parentJoin.getParent();
}
- return true;
+ if (result.getParent() == groupNode) {
+ //can't be pushed as we are already at the direct child
+ return null;
+ }
+ return result;
}
- private void filterJoinColumns(Set<Expression> stagedGroupingSymbols,
+ /**
+ * @return true if the filtered expressions contain outside groups
+ */
+ private boolean filterExpressions(Set<Expression> stagedGroupingSymbols,
Set<GroupSymbol> groups,
- List<? extends Expression> symbols) {
+ Collection<? extends Expression> symbols, boolean wholeExpression) {
+ boolean result = false;
for (Expression ex : symbols) {
- if (groups.containsAll(GroupsUsedByElementsVisitor.getGroups(ex))) {
+ Set<GroupSymbol> groups2 = GroupsUsedByElementsVisitor.getGroups(ex);
+ if (!Collections.disjoint(groups, groups2)) {
+ if (!result) {
+ boolean containsAll = groups.containsAll(groups2);
+ if (!wholeExpression && !containsAll) {
+ //collect only matching subexpressions - but the best that we'll currently do is elementsymbols
+ filterExpressions(stagedGroupingSymbols, groups, ElementCollectorVisitor.getElements(ex, true), true);
+ continue;
+ }
+ result = !containsAll;
+ }
stagedGroupingSymbols.add(SymbolMap.getExpression(ex));
}
}
+ return result;
}
private <T extends Expression> Map<PlanNode, List<T>> createNodeMapping(PlanNode groupNode,
Modified: trunk/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java
===================================================================
--- trunk/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java 2012-09-18 19:18:44 UTC (rev 4456)
+++ trunk/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java 2012-09-20 19:43:05 UTC (rev 4457)
@@ -410,6 +410,39 @@
helpProcess(plan, dataManager, expected);
}
+ @Test public void testMultiJoinCriteria() throws Exception {
+ String sql = "SELECT count(t2.e4) as s FROM pm1.g1 as t1, pm1.g2 as t2, pm1.g3 as t3 WHERE t1.e1 = t2.e1 and t2.e2 = t3.e2 and t1.e3 || t2.e3 = t3.e3"; //$NON-NLS-1$
+
+ List[] expected = new List[] {
+ Arrays.asList(0)
+ };
+
+ FakeDataManager dataManager = new FakeDataManager();
+ sampleData1(dataManager);
+
+ ProcessorPlan plan = helpGetPlan(sql, RealMetadataFactory.example1Cached());
+
+ helpProcess(plan, dataManager, expected);
+ }
+
+ @Test public void testMultiJoinGroupBy() throws Exception {
+ String sql = "SELECT count(t2.e4) as s, t1.e3 || t2.e3 FROM pm1.g1 as t1, pm1.g2 as t2, pm1.g3 as t3 WHERE t1.e1 = t2.e1 and t2.e2 = t3.e2 GROUP BY t1.e3 || t2.e3"; //$NON-NLS-1$
+
+ List[] expected = new List[] {
+ Arrays.asList(9, "falsefalse"),
+ Arrays.asList(2, "falsetrue"),
+ Arrays.asList(4, "truefalse"),
+ Arrays.asList(1, "truetrue"),
+ };
+
+ FakeDataManager dataManager = new FakeDataManager();
+ sampleData1(dataManager);
+
+ ProcessorPlan plan = helpGetPlan(sql, RealMetadataFactory.example1Cached());
+
+ helpProcess(plan, dataManager, expected);
+ }
+
@Test public void testArrayAggOrderByPersistence() throws Exception {
// Create query
String sql = "SELECT array_agg(e2 order by e1) from pm1.g1 group by e3"; //$NON-NLS-1$
More information about the teiid-commits
mailing list