TEIID-2967 correcting several aspects of sort optimization

teiid · May 21, 2014 · ed7e74c · ed7e74c
1 parent b1a42a8
commit ed7e74c
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 17 deletions.
diff --git a/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java b/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java
@@ -219,9 +219,9 @@ private PlanNode checkForProjectOptimization(PlanNode node, PlanNode root,
 		boolean raiseAccess = false;
 		//special check for unrelated order by compensation
 		if (projectNode.getType() == NodeConstants.Types.ACCESS && RuleRaiseAccess.canRaiseOverSort(projectNode, metadata, capFinder, node, record, true)) {
-			projectNode = NodeEditor.findNodePreOrder(projectNode, NodeConstants.Types.PROJECT, NodeConstants.Types.SOURCE);
+			projectNode = NodeEditor.findNodePreOrder(projectNode, NodeConstants.Types.PROJECT, NodeConstants.Types.SOURCE | NodeConstants.Types.SET_OP);
 			if (projectNode == null) {
-				return root; //shouldn't happen
+				return root; //no interviening project
 			}
 			raiseAccess = true;
 		} else if (projectNode.getType() == NodeConstants.Types.PROJECT && projectNode.getFirstChild() != null) {

diff --git a/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java b/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java
@@ -331,14 +331,13 @@ private void distributeLimit(PlanNode limitNode, PlanNode setOp,
 					}
 				} else {
 					if (branch.getType() == NodeConstants.Types.ACCESS &&
-							(!RuleRaiseAccess.canRaiseOverSort(branch, metadata, capFinder, newSort, null, false)
-									|| !CapabilitiesUtil.supportsRowLimit(RuleRaiseAccess.getModelIDFromAccess(branch, metadata), metadata, capFinder))) {
-						//TODO - once we support sorted sublist processing, then we'll want to push
-						continue outer;
+							RuleRaiseAccess.canRaiseOverSort(branch, metadata, capFinder, newSort, null, false)) {
+						branch.getFirstChild().addAsParent(newSort);
+					} else {
+						//TODO: if the limit is too large we shouldn't add it in here
+						branch.addAsParent(newSort);
+						branch = newSort;
 					}
-					//TODO: a better check to see if limit is supported - as it may not be pushed
-					branch.addAsParent(newSort);
-					branch = newSort;
 				}
 				addBranchLimit(limitNode, limitNodes, metadata, parentLimit, parentOffset, branch);
 			}

diff --git a/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java b/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java
@@ -402,13 +402,16 @@ static boolean canRaiseOverSort(PlanNode accessNode,
             }
         }
 
+        boolean isSet = false;
         if (accessNode.getLastChild() != null) {
             //check to see if the sort applies to a union
             if (accessNode.getLastChild().getType() == NodeConstants.Types.SET_OP) {
-                return CapabilitiesUtil.supportsSetQueryOrderBy(modelID, metadata, capFinder);
-            }
-            //check to see the plan is not in a consistent state to have a sort applied
-            if (accessNode.getLastChild().getType() == NodeConstants.Types.TUPLE_LIMIT) {
+            	isSet = true;
+                if (!CapabilitiesUtil.supportsSetQueryOrderBy(modelID, metadata, capFinder)) {
+                	return false;
+                }
+            } else if (accessNode.getLastChild().getType() == NodeConstants.Types.TUPLE_LIMIT) { 
+            	//check to see the plan is not in a consistent state to have a sort applied	
                 return false;
             }
         }
@@ -418,11 +421,11 @@ static boolean canRaiseOverSort(PlanNode accessNode,
         }
 
         // If model supports the support constant parameter, then move access node
-        if (!CapabilitiesUtil.supportsOrderBy(modelID, metadata, capFinder)) {
+        if (!isSet && !CapabilitiesUtil.supportsOrderBy(modelID, metadata, capFinder)) {
         	return false;
         }
 
-        if (parentNode.hasBooleanProperty(NodeConstants.Info.UNRELATED_SORT) 
+        if (!isSet && parentNode.hasBooleanProperty(NodeConstants.Info.UNRELATED_SORT) 
         		&& !CapabilitiesUtil.supports(Capability.QUERY_ORDERBY_UNRELATED, modelID, metadata, capFinder)
         		&& NodeEditor.findParent(accessNode, NodeConstants.Types.PROJECT, NodeConstants.Types.SOURCE) == null
         		&& !compensateForUnrelated) {

diff --git a/engine/src/test/java/org/teiid/query/optimizer/TestSortOptimization.java b/engine/src/test/java/org/teiid/query/optimizer/TestSortOptimization.java
@@ -31,6 +31,7 @@
 import org.junit.Test;
 import org.teiid.core.TeiidComponentException;
 import org.teiid.core.TeiidProcessingException;
+import org.teiid.query.metadata.QueryMetadataInterface;
 import org.teiid.query.optimizer.TestOptimizer.ComparisonMode;
 import org.teiid.query.optimizer.TestOptimizer.DupRemoveSortNode;
 import org.teiid.query.optimizer.capabilities.BasicSourceCapabilities;
@@ -466,5 +467,23 @@ public class TestSortOptimization {
             });
         checkNodeTypes(plan, new int[] {1}, new Class[] {DupRemoveSortNode.class});
     }
+
+    @Test public void testUnionWithAggregation() throws Exception{
+    	QueryMetadataInterface metadata = RealMetadataFactory.fromDDL("create foreign table items (item_id varchar)", "x", "y");
+
+        String sql = "select FOO.SOURCE SOURCE, FOO.FOO_ID FOO_ID from ("
+                + "(select 'X' SOURCE, ITEMS.ITEM_ID FOO_ID from ITEMS ITEMS group by ITEMS.ITEM_ID) "
+                + "union all (select 'Y' SOURCE, ITEMS.ITEM_ID FOO_ID from ITEMS ITEMS) union all"
+                + " (select 'Z' SOURCE, '123' FOO_ID) ) FOO order by FOO_ID desc limit 50";
+
+        BasicSourceCapabilities caps = TestOptimizer.getTypicalCapabilities();
+        caps.setCapabilitySupport(Capability.ROW_LIMIT, true);
+        caps.setCapabilitySupport(Capability.QUERY_UNION, true);
+        caps.setCapabilitySupport(Capability.QUERY_GROUP_BY, true);
+        caps.setCapabilitySupport(Capability.QUERY_SET_ORDER_BY, true);
+
+        helpPlan(sql, metadata, null, new DefaultCapabilitiesFinder(caps), 
+                new String[] {"SELECT 'X' AS c_0, g_1.item_id AS c_1 FROM y.items AS g_1 GROUP BY g_1.item_id UNION ALL SELECT 'Y' AS c_0, g_0.item_id AS c_1 FROM y.items AS g_0 ORDER BY c_1 DESC LIMIT 50"}, ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$
+    }
 
 }
diff --git a/engine/src/test/java/org/teiid/query/optimizer/TestUnionPlanning.java b/engine/src/test/java/org/teiid/query/optimizer/TestUnionPlanning.java
@@ -474,7 +474,7 @@ public void testUnionPushDown3() {
                 0,      // MergeJoinStrategy
                 0,      // Null
                 0,      // PlanExecution
-                1,      // Project
+                0,      // Project
                 0,      // Select
                 1,      // Sort
                 1       // UnionAll
@@ -508,7 +508,7 @@ public void testUnionPushDown3() {
                 0,      // PlanExecution
                 0,      // Project
                 0,      // Select
-                1,      // Sort
+                2,      // Sort
                 1       // UnionAll
             });