[teiid-commits] teiid SVN: r2894 - in trunk/engine/src: test/java/org/teiid/query/optimizer and 1 other directories.
teiid-commits at lists.jboss.org
teiid-commits at lists.jboss.org
Tue Feb 1 10:31:51 EST 2011
Author: shawkins
Date: 2011-02-01 10:31:51 -0500 (Tue, 01 Feb 2011)
New Revision: 2894
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/RuleRemoveOptionalJoins.java
trunk/engine/src/test/java/org/teiid/query/optimizer/TestAggregatePushdown.java
trunk/engine/src/test/java/org/teiid/query/processor/TestProcessor.java
trunk/engine/src/test/java/org/teiid/query/processor/TestVirtualDepJoin.java
Log:
TEIID-1458 fix to ensure aggregates that need computed after a join and are cardinality dependent return the proper value
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 2011-02-01 15:26:05 UTC (rev 2893)
+++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushAggregates.java 2011-02-01 15:31:51 UTC (rev 2894)
@@ -530,8 +530,11 @@
CapabilitiesFinder capFinder) throws TeiidComponentException,
QueryMetadataException, QueryPlannerException {
- Map<PlanNode, List<AggregateSymbol>> aggregateMap = createNodeMapping(groupNode, allAggregates);
- Map<PlanNode, List<SingleElementSymbol>> groupingMap = createNodeMapping(groupNode, groupingExpressions);
+ Map<PlanNode, List<AggregateSymbol>> aggregateMap = createNodeMapping(groupNode, allAggregates, true);
+ if (aggregateMap == null) {
+ return;
+ }
+ Map<PlanNode, List<SingleElementSymbol>> groupingMap = createNodeMapping(groupNode, groupingExpressions, false);
Set<PlanNode> possibleTargetNodes = new HashSet<PlanNode>(aggregateMap.keySet());
possibleTargetNodes.addAll(groupingMap.keySet());
@@ -707,26 +710,25 @@
}
private <T extends SingleElementSymbol> Map<PlanNode, List<T>> createNodeMapping(PlanNode groupNode,
- Collection<T> expressions) {
+ Collection<T> expressions, boolean aggs) {
Map<PlanNode, List<T>> result = new HashMap<PlanNode, List<T>>();
if (expressions == null) {
return result;
}
for (T aggregateSymbol : expressions) {
- if (aggregateSymbol instanceof AggregateSymbol) {
- AggregateSymbol partitionAgg = (AggregateSymbol)aggregateSymbol;
- if (partitionAgg.isDistinct()) {
- continue; //currently we cann't consider distinct aggs
- }
- }
-
+ if (aggs && ((AggregateSymbol)aggregateSymbol).getExpression() == null) {
+ return null; //count(*) is not yet handled. a general approach would be count(*) => count(r.col) * count(l.col), but the logic here assumes a simpler initial mapping
+ }
Set<GroupSymbol> groups = GroupsUsedByElementsVisitor.getGroups(aggregateSymbol);
if (groups.isEmpty()) {
- continue;
+ continue;
}
PlanNode originatingNode = FrameUtil.findOriginatingNode(groupNode, groups);
if (originatingNode == null) {
- continue;
+ if (aggs) {
+ return null; //should never happen
+ }
+ continue;
}
PlanNode parentAccess = NodeEditor.findParent(originatingNode, NodeConstants.Types.ACCESS, NodeConstants.Types.GROUP);
@@ -739,8 +741,18 @@
}
if (originatingNode.getParent() == groupNode) {
+ //anything logically applied after the join and is
+ //dependent upon the cardinality prevents us from optimizing.
+ if (aggs && RuleRemoveOptionalJoins.isCardinalityDependent((AggregateSymbol)aggregateSymbol)) {
+ return null;
+ }
continue;
}
+
+ if (aggs && ((AggregateSymbol)aggregateSymbol).isDistinct()) {
+ //TODO: support distinct
+ continue;
+ }
List<T> symbols = result.get(originatingNode);
if (symbols == null) {
Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java
===================================================================
--- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java 2011-02-01 15:26:05 UTC (rev 2893)
+++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java 2011-02-01 15:31:51 UTC (rev 2894)
@@ -274,18 +274,29 @@
static boolean areAggregatesCardinalityDependent(Set<AggregateSymbol> aggs) {
for (AggregateSymbol aggregateSymbol : aggs) {
- switch (aggregateSymbol.getAggregateFunction()) {
- case COUNT:
- case AVG:
- case STDDEV_POP:
- case STDDEV_SAMP:
- case VAR_POP:
- case VAR_SAMP:
+ if (isCardinalityDependent(aggregateSymbol)) {
return true;
}
}
return false;
}
+
+ static boolean isCardinalityDependent(AggregateSymbol aggregateSymbol) {
+ if (aggregateSymbol.isDistinct()) {
+ return false;
+ }
+ switch (aggregateSymbol.getAggregateFunction()) {
+ case COUNT:
+ case AVG:
+ case STDDEV_POP:
+ case STDDEV_SAMP:
+ case VAR_POP:
+ case VAR_SAMP:
+ case SUM:
+ return true;
+ }
+ return false;
+ }
public String toString() {
return "RuleRemoveOptionalJoins"; //$NON-NLS-1$
Modified: trunk/engine/src/test/java/org/teiid/query/optimizer/TestAggregatePushdown.java
===================================================================
--- trunk/engine/src/test/java/org/teiid/query/optimizer/TestAggregatePushdown.java 2011-02-01 15:26:05 UTC (rev 2893)
+++ trunk/engine/src/test/java/org/teiid/query/optimizer/TestAggregatePushdown.java 2011-02-01 15:31:51 UTC (rev 2894)
@@ -104,7 +104,7 @@
capFinder.addCapabilities("BQT1", caps); //$NON-NLS-1$
capFinder.addCapabilities("BQT2", caps); //$NON-NLS-1$
- String sql = "SELECT a12.intkey, MAX(a12.stringkey), SUM(a11.intnum+a12.intnum) FROM bqt1.smalla AS a11 INNER JOIN bqt2.smalla AS a12 ON a11.stringkey = a12.stringkey GROUP BY a12.intkey"; //$NON-NLS-1$
+ String sql = "SELECT a12.intkey, MAX(a12.stringkey), MIN(a11.intnum+a12.intnum) FROM bqt1.smalla AS a11 INNER JOIN bqt2.smalla AS a12 ON a11.stringkey = a12.stringkey GROUP BY a12.intkey"; //$NON-NLS-1$
ProcessorPlan plan = TestOptimizer.helpPlan(sql, FakeMetadataFactory.exampleBQTCached(), null, capFinder,
new String[] {"SELECT g_0.stringkey, g_0.intkey, g_0.intnum FROM bqt2.smalla AS g_0 GROUP BY g_0.stringkey, g_0.intkey, g_0.intnum", "SELECT g_0.stringkey, g_0.intnum FROM bqt1.smalla AS g_0"}, TestOptimizer.ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ //$NON-NLS-2$
@@ -126,6 +126,37 @@
});
}
+ @Test public void testAggregateOfJoinExpression1() throws Exception {
+ FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder();
+ BasicSourceCapabilities caps = getAggregateCapabilities();
+ caps.setCapabilitySupport(Capability.QUERY_FROM_GROUP_ALIAS, true);
+ caps.setCapabilitySupport(Capability.QUERY_ORDERBY, false);
+ caps.setFunctionSupport("convert", true); //$NON-NLS-1$
+ capFinder.addCapabilities("BQT1", caps); //$NON-NLS-1$
+ capFinder.addCapabilities("BQT2", caps); //$NON-NLS-1$
+
+ String sql = "SELECT a12.intkey, MAX(a12.stringkey), SUM(a11.intnum+a12.intnum) FROM bqt1.smalla AS a11 INNER JOIN bqt2.smalla AS a12 ON a11.stringkey = a12.stringkey GROUP BY a12.intkey"; //$NON-NLS-1$
+ ProcessorPlan plan = TestOptimizer.helpPlan(sql, FakeMetadataFactory.exampleBQTCached(), null, capFinder,
+ new String[] {"SELECT g_0.stringkey, g_0.intkey, g_0.intnum FROM bqt2.smalla AS g_0", "SELECT g_0.stringkey, g_0.intnum FROM bqt1.smalla AS g_0"}, TestOptimizer.ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ //$NON-NLS-2$
+
+ TestOptimizer.checkNodeTypes(plan, new int[] {
+ 2, // Access
+ 0, // DependentAccess
+ 0, // DependentSelect
+ 0, // DependentProject
+ 0, // DupRemove
+ 1, // Grouping
+ 0, // NestedLoopJoinStrategy
+ 1, // MergeJoinStrategy
+ 0, // Null
+ 0, // PlanExecution
+ 1, // Project
+ 0, // Select
+ 0, // Sort
+ 0 // UnionAll
+ });
+ }
+
/**
* Note that even though this grouping is join invariant, we still do not remove the top level group by
* since we are not checking the uniqueness of the x side join expressions
@@ -205,7 +236,7 @@
ProcessorPlan plan = TestOptimizer.helpPlan(sql,
metadata,
null, getAggregatesFinder(),
- new String[] {"SELECT DISTINCT g_0.p_productid AS c_0 FROM m2.product AS g_0 WHERE g_0.p_divid = 100 ORDER BY c_0", "SELECT g_0.o_productid AS c_0, g_0.o_dealerid AS c_1, SUM(g_0.o_amount) AS c_2 FROM m1.\"order\" AS g_0, m1.dealer AS g_1 WHERE (g_0.o_dealerid = g_1.d_dealerid) AND (g_1.d_state = 'CA') AND (g_0.o_productid IN (<dependent values>)) GROUP BY g_0.o_productid, g_0.o_dealerid ORDER BY c_0"}, //$NON-NLS-1$ //$NON-NLS-2$
+ new String[] {"SELECT g_0.p_productid AS c_0 FROM m2.product AS g_0 WHERE g_0.p_divid = 100 ORDER BY c_0", "SELECT g_0.o_productid AS c_0, g_0.o_dealerid AS c_1, SUM(g_0.o_amount) AS c_2 FROM m1.\"order\" AS g_0, m1.dealer AS g_1 WHERE (g_0.o_dealerid = g_1.d_dealerid) AND (g_1.d_state = 'CA') AND (g_0.o_productid IN (<dependent values>)) GROUP BY g_0.o_productid, g_0.o_dealerid ORDER BY c_0"}, //$NON-NLS-1$ //$NON-NLS-2$
TestOptimizer.ComparisonMode.EXACT_COMMAND_STRING );
TestOptimizer.checkNodeTypes(plan, new int[] {
@@ -235,7 +266,7 @@
ProcessorPlan plan = TestOptimizer.helpPlan(sql,
metadata,
null, getAggregatesFinder(),
- new String[] {"SELECT DISTINCT g_0.p_productid AS c_0 FROM m2.product AS g_0 WHERE g_0.p_divid = 100 ORDER BY c_0", "SELECT g_0.o_productid AS c_0, g_0.o_dealerid AS c_1, MAX(g_0.o_amount) AS c_2, SUM(g_0.o_amount) AS c_3 FROM m1.\"order\" AS g_0, m1.dealer AS g_1 WHERE (g_0.o_dealerid = g_1.d_dealerid) AND (g_1.d_state = 'CA') AND (g_0.o_productid IN (<dependent values>)) GROUP BY g_0.o_productid, g_0.o_dealerid ORDER BY c_0"}, //$NON-NLS-1$ //$NON-NLS-2$
+ new String[] {"SELECT g_0.p_productid AS c_0 FROM m2.product AS g_0 WHERE g_0.p_divid = 100 ORDER BY c_0", "SELECT g_0.o_productid AS c_0, g_0.o_dealerid AS c_1, MAX(g_0.o_amount) AS c_2, SUM(g_0.o_amount) AS c_3 FROM m1.\"order\" AS g_0, m1.dealer AS g_1 WHERE (g_0.o_dealerid = g_1.d_dealerid) AND (g_1.d_state = 'CA') AND (g_0.o_productid IN (<dependent values>)) GROUP BY g_0.o_productid, g_0.o_dealerid ORDER BY c_0"}, //$NON-NLS-1$ //$NON-NLS-2$
TestOptimizer.ComparisonMode.EXACT_COMMAND_STRING );
TestOptimizer.checkNodeTypes(plan, new int[] {
@@ -377,8 +408,7 @@
}
/**
- * Test to ensure count(*) isn't mistakenly pushed to either side, but that
- * grouping can still be.
+ * Test to ensure count(*) isn't mistakenly pushed to either side
*/
@Test public void testCase5724() {
FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder();
@@ -392,7 +422,7 @@
"select count(*), a.intnum from bqt1.smalla as a, bqt2.smallb as b where a.intkey = b.intkey group by a.intnum", //$NON-NLS-1$
metadata, null, capFinder,
new String[] {
- "SELECT a.intkey, a.intnum FROM bqt1.smalla AS a group by a.intkey, a.intnum", "SELECT b.intkey FROM bqt2.smallb AS b"}, //$NON-NLS-1$ //$NON-NLS-2$
+ "SELECT a.intkey, a.intnum FROM bqt1.smalla AS a", "SELECT b.intkey FROM bqt2.smallb AS b"}, //$NON-NLS-1$ //$NON-NLS-2$
true);
TestOptimizer.checkNodeTypes(plan, new int[] {
Modified: trunk/engine/src/test/java/org/teiid/query/processor/TestProcessor.java
===================================================================
--- trunk/engine/src/test/java/org/teiid/query/processor/TestProcessor.java 2011-02-01 15:26:05 UTC (rev 2893)
+++ trunk/engine/src/test/java/org/teiid/query/processor/TestProcessor.java 2011-02-01 15:31:51 UTC (rev 2894)
@@ -69,6 +69,7 @@
import org.teiid.query.metadata.TempMetadataStore;
import org.teiid.query.optimizer.FakeFunctionMetadataSource;
import org.teiid.query.optimizer.QueryOptimizer;
+import org.teiid.query.optimizer.TestAggregatePushdown;
import org.teiid.query.optimizer.TestOptimizer;
import org.teiid.query.optimizer.TestRuleRaiseNull;
import org.teiid.query.optimizer.TestOptimizer.ComparisonMode;
@@ -96,6 +97,7 @@
import org.teiid.query.unittest.FakeMetadataFactory;
import org.teiid.query.unittest.FakeMetadataObject;
import org.teiid.query.unittest.FakeMetadataStore;
+import org.teiid.query.unittest.RealMetadataFactory;
import org.teiid.query.unittest.TimestampUtil;
import org.teiid.query.util.CommandContext;
import org.teiid.query.validator.Validator;
Modified: trunk/engine/src/test/java/org/teiid/query/processor/TestVirtualDepJoin.java
===================================================================
--- trunk/engine/src/test/java/org/teiid/query/processor/TestVirtualDepJoin.java 2011-02-01 15:26:05 UTC (rev 2893)
+++ trunk/engine/src/test/java/org/teiid/query/processor/TestVirtualDepJoin.java 2011-02-01 15:31:51 UTC (rev 2894)
@@ -390,8 +390,8 @@
List<String> expectedQueries = new ArrayList<String>(6);
for (int i = 0; i < 3; i++) {
- expectedQueries.add("SELECT DISTINCT g_0.id AS c_0, g_0.first AS c_1, g_0.last AS c_2 FROM CustomerMaster.Customers AS g_0 WHERE g_0.first = 'Miles' ORDER BY c_0"); //$NON-NLS-1$
- expectedQueries.add("SELECT DISTINCT g_0.id AS c_0, g_0.amount AS c_1 FROM Europe.CustAccts AS g_0 WHERE g_0.id = 100 ORDER BY c_0"); //$NON-NLS-1$
+ expectedQueries.add("SELECT g_0.id AS c_0, g_0.first AS c_1, g_0.last AS c_2 FROM CustomerMaster.Customers AS g_0 WHERE g_0.first = 'Miles' ORDER BY c_0"); //$NON-NLS-1$
+ expectedQueries.add("SELECT g_0.id AS c_0, g_0.amount AS c_1 FROM Europe.CustAccts AS g_0 WHERE g_0.id = 100 ORDER BY c_0"); //$NON-NLS-1$
}
assertEquals(expectedQueries, dataManager.getQueries());
More information about the teiid-commits
mailing list