Author: shawkins Date: 2010-03-02 22:29:06 -0500 (Tue, 02 Mar 2010) New Revision: 1888 Modified: trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleAssignOutputElements.java trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleMergeVirtual.java trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestExpressionsInGroupBy.java trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestLimit.java trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestOptionalJoins.java trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestRuleMergeVirtual.java trunk/engine/src/test/java/com/metamatrix/query/processor/TestProcessor.java Log: TEIID-1007 enhancing the rulemergevirtual logic to check for columns used in a group by clause Modified: trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleAssignOutputElements.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleAssignOutputElements.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleAssignOutputElements.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -162,7 +162,26 @@ case NodeConstants.Types.SOURCE: { outputElements = (List<SingleElementSymbol>)determineSourceOutput(root, outputElements); root.setProperty(NodeConstants.Info.OUTPUT_COLS, outputElements); - assignOutputElements(root.getFirstChild(), filterVirtualElements(root, outputElements, metadata), metadata, capFinder, rules, analysisRecord, context); + List<SingleElementSymbol> childElements = filterVirtualElements(root, outputElements, metadata); + SymbolMap symbolMap = (SymbolMap)root.getProperty(NodeConstants.Info.SYMBOL_MAP); + int size = symbolMap.asMap().size(); + symbolMap.asUpdatableMap().keySet().retainAll(outputElements); + //if we have removed projected symbols, then we need to update the sort columns + if (size > outputElements.size()) { + PlanNode sortNode = NodeEditor.findNodePreOrder(root, NodeConstants.Types.SORT, NodeConstants.Types.PROJECT); + if (sortNode != null && !sortNode.hasBooleanProperty(NodeConstants.Info.UNRELATED_SORT)) { + List<Expression> symbolOrder = symbolMap.getValues(); + OrderBy elements = (OrderBy) sortNode.getProperty(NodeConstants.Info.SORT_ORDER); + for (OrderByItem item : elements.getOrderByItems()) { + int position = symbolOrder.indexOf(SymbolMap.getExpression(item.getSymbol())); + item.setExpressionPosition(position); + if (position == -1) { + sortNode.setProperty(NodeConstants.Info.UNRELATED_SORT, true); + } + } + } + } + assignOutputElements(root.getFirstChild(), childElements, metadata, capFinder, rules, analysisRecord, context); break; } case NodeConstants.Types.SET_OP: { @@ -228,31 +247,7 @@ SymbolMap symbolMap = (SymbolMap) root.getProperty(NodeConstants.Info.SYMBOL_MAP); return symbolMap.getKeys(); } - PlanNode limit = NodeEditor.findNodePreOrder(root, NodeConstants.Types.TUPLE_LIMIT, NodeConstants.Types.PROJECT); - if (limit == null) { - return outputElements; - } - //reset the output elements to be the output columns + what's required by the sort - PlanNode sort = NodeEditor.findNodePreOrder(limit, NodeConstants.Types.SORT, NodeConstants.Types.PROJECT); - if (sort == null) { - return outputElements; - } - OrderBy sortOrder = (OrderBy)sort.getProperty(NodeConstants.Info.SORT_ORDER); - List<SingleElementSymbol> topCols = FrameUtil.findTopCols(sort); - - SymbolMap symbolMap = (SymbolMap)root.getProperty(NodeConstants.Info.SYMBOL_MAP); - - List<ElementSymbol> symbolOrder = symbolMap.getKeys(); - - for (OrderByItem item : sortOrder.getOrderByItems()) { - final Expression expr = item.getSymbol(); - int index = topCols.indexOf(expr); - ElementSymbol symbol = symbolOrder.get(index); - if (!outputElements.contains(symbol)) { - outputElements.add(symbol); - } - } - return outputElements; + return outputElements; } /** Modified: trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleMergeVirtual.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleMergeVirtual.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/RuleMergeVirtual.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -133,15 +133,20 @@ } PlanNode parentGroup = NodeEditor.findParent(frame, NodeConstants.Types.GROUP, NodeConstants.Types.SOURCE); + List<SingleElementSymbol> groupCols = null; + if (parentGroup != null) { + groupCols = (List<SingleElementSymbol>)parentGroup.getProperty(NodeConstants.Info.GROUP_COLS); + } - if (!checkProjectedSymbols(projectNode, virtualGroup, parentJoin, parentGroup, metadata)) { + SymbolMap symbolMap = (SymbolMap)frame.getProperty(NodeConstants.Info.SYMBOL_MAP); + + if (!checkProjectedSymbols(projectNode, virtualGroup, parentJoin, groupCols, symbolMap, metadata)) { return root; } // Otherwise merge should work // Convert parent frame before merge - SymbolMap symbolMap = (SymbolMap)frame.getProperty(NodeConstants.Info.SYMBOL_MAP); FrameUtil.convertFrame(frame, virtualGroup, FrameUtil.findJoinSourceNode(projectNode).getGroups(), symbolMap.asMap(), metadata); PlanNode parentBottom = frame.getParent(); @@ -195,7 +200,7 @@ return root; } - List<? extends SingleElementSymbol> requiredElements = RuleAssignOutputElements.determineSourceOutput(frame, new ArrayList<SingleElementSymbol>()); + List<? extends SingleElementSymbol> requiredElements = getNecessaryOutput(frame); List<SingleElementSymbol> selectSymbols = (List<SingleElementSymbol>)parentProject.getProperty(NodeConstants.Info.PROJECT_COLS); // check that it only performs simple projection and that all required symbols are projected @@ -267,6 +272,42 @@ return root; } + + private static List<? extends SingleElementSymbol> getNecessaryOutput(PlanNode root) { + List<SingleElementSymbol> outputElements = new ArrayList<SingleElementSymbol>(); + List<? extends SingleElementSymbol> result = RuleAssignOutputElements.determineSourceOutput(root, outputElements); + if (!result.isEmpty()) { + return result; + } + PlanNode limit = NodeEditor.findNodePreOrder(root, NodeConstants.Types.TUPLE_LIMIT, NodeConstants.Types.PROJECT); + if (limit == null) { + return outputElements; + } + //reset the output elements to be the output columns + what's required by the sort + PlanNode sort = NodeEditor.findNodePreOrder(limit, NodeConstants.Types.SORT, NodeConstants.Types.PROJECT); + if (sort == null) { + return outputElements; + } + OrderBy sortOrder = (OrderBy)sort.getProperty(NodeConstants.Info.SORT_ORDER); + List<SingleElementSymbol> topCols = FrameUtil.findTopCols(sort); + + SymbolMap symbolMap = (SymbolMap)root.getProperty(NodeConstants.Info.SYMBOL_MAP); + + List<ElementSymbol> symbolOrder = symbolMap.getKeys(); + + for (OrderByItem item : sortOrder.getOrderByItems()) { + final Expression expr = item.getSymbol(); + int index = topCols.indexOf(expr); + if (index < 0) { + continue; + } + ElementSymbol symbol = symbolOrder.get(index); + if (!outputElements.contains(symbol)) { + outputElements.add(symbol); + } + } + return outputElements; + } /** * Check to ensure that we are not projecting a subquery or null dependent expressions @@ -274,7 +315,8 @@ private static boolean checkProjectedSymbols(PlanNode projectNode, GroupSymbol virtualGroup, PlanNode parentJoin, - PlanNode parentGroup, + List<SingleElementSymbol> parentGroupingCols, + SymbolMap symbolMap, QueryMetadataInterface metadata) { List<SingleElementSymbol> selectSymbols = (List<SingleElementSymbol>)projectNode.getProperty(NodeConstants.Info.PROJECT_COLS); @@ -302,7 +344,9 @@ } //TODO: in each of the cases below, check to see if the offending projected symbol is actually used in the upper frame - for (SingleElementSymbol symbol : selectSymbols) { + List<ElementSymbol> virtualElements = symbolMap.getKeys(); + for (int i = 0; i < selectSymbols.size(); i++) { + SingleElementSymbol symbol = selectSymbols.get(i); Collection scalarSubqueries = ValueIteratorProviderCollectorVisitor.getValueIteratorProviders(symbol); if (!scalarSubqueries.isEmpty()) { return false; @@ -310,7 +354,7 @@ if (checkForNullDependent && JoinUtil.isNullDependent(metadata, groups, SymbolMap.getExpression(symbol))) { return false; } - if (parentGroup != null && !(SymbolMap.getExpression(symbol) instanceof SingleElementSymbol)) { + if (parentGroupingCols != null && !(SymbolMap.getExpression(symbol) instanceof SingleElementSymbol) && parentGroupingCols.contains(virtualElements.get(i))) { return false; } // TEIID-16: We do not want to merge a non-deterministic scalar function Modified: trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestExpressionsInGroupBy.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestExpressionsInGroupBy.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestExpressionsInGroupBy.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -205,7 +205,7 @@ 0, // MergeJoinStrategy 0, // Null 0, // PlanExecution - 2, // Project + 1, // Project 0, // Select 0, // Sort 0 // UnionAll Modified: trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestLimit.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestLimit.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestLimit.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -865,7 +865,7 @@ public void testSortWithLimitInlineView() { String sql = "select e1 from (select pm1.g1.e1, pm1.g1.e2 from pm1.g1 order by pm1.g1.e1, pm1.g1.e2 limit 1) x"; //$NON-NLS-1$ - ProcessorPlan plan = TestOptimizer.helpPlan(sql, FakeMetadataFactory.example1Cached(), new String[] {"SELECT g_0.e1 AS c_0, g_0.e2 AS c_1 FROM pm1.g1 AS g_0 ORDER BY c_0, c_1"}); //$NON-NLS-1$ + ProcessorPlan plan = TestOptimizer.helpPlan(sql, FakeMetadataFactory.example1Cached(), new String[] {"SELECT g_0.e1 AS c_0 FROM pm1.g1 AS g_0 ORDER BY c_0, g_0.e2"}); //$NON-NLS-1$ TestOptimizer.checkNodeTypes(plan, new int[] { 1, // Access Modified: trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestOptionalJoins.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestOptionalJoins.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestOptionalJoins.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -287,7 +287,7 @@ @Test public void testOptionalJoinWithNestedOrderBy() { ProcessorPlan plan = TestOptimizer.helpPlan("SELECT pm1.g3.e1 FROM pm1.g3 inner join (select pm1.g2.e1, pm1.g1.e2 as y from /* optional */ pm1.g1 inner join pm1.g2 on pm1.g1.e1 = pm1.g2.e1 order by pm1.g2.e1 limit 10000) AS x on pm1.g3.e2=x.y", FakeMetadataFactory.example1Cached(), //$NON-NLS-1$ - new String[] {"SELECT g_0.e2 AS c_0, g_0.e1 AS c_1 FROM pm1.g3 AS g_0 ORDER BY c_0", "SELECT g_0.e2 AS c_0, g_1.e1 AS c_1 FROM pm1.g1 AS g_0, pm1.g2 AS g_1 WHERE g_0.e1 = g_1.e1 ORDER BY c_1"} ); //$NON-NLS-1$ //$NON-NLS-2$ + new String[] {"SELECT g_0.e2 AS c_0, g_0.e1 AS c_1 FROM pm1.g3 AS g_0 ORDER BY c_0", "SELECT g_0.e2 AS c_0 FROM pm1.g1 AS g_0, pm1.g2 AS g_1 WHERE g_0.e1 = g_1.e1 ORDER BY g_1.e1"} ); //$NON-NLS-1$ //$NON-NLS-2$ TestOptimizer.checkNodeTypes(plan, new int[] { 2, // Access Modified: trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestRuleMergeVirtual.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestRuleMergeVirtual.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/test/java/com/metamatrix/query/optimizer/TestRuleMergeVirtual.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -22,11 +22,13 @@ package com.metamatrix.query.optimizer; +import static junit.framework.Assert.*; + import org.junit.Test; -import static junit.framework.Assert.*; - +import com.metamatrix.query.optimizer.TestOptimizer.ComparisonMode; import com.metamatrix.query.optimizer.capabilities.BasicSourceCapabilities; +import com.metamatrix.query.optimizer.capabilities.DefaultCapabilitiesFinder; import com.metamatrix.query.optimizer.capabilities.FakeCapabilitiesFinder; import com.metamatrix.query.optimizer.capabilities.SourceCapabilities.Capability; import com.metamatrix.query.processor.ProcessorPlan; @@ -303,5 +305,16 @@ SortNode node = (SortNode)plan.getRootNode(); assertTrue("Alias was not accounted for in sort node", node.getElements().containsAll(node.getSortElements())); //$NON-NLS-1$ } + + @Test public void testMergeImplicitGroupBy() throws Exception { + BasicSourceCapabilities caps = TestAggregatePushdown.getAggregateCapabilities(); + caps.setFunctionSupport("+", true); //$NON-NLS-1$ + ProcessorPlan plan = TestOptimizer.helpPlan("SELECT x FROM (SELECT min(y), max(x) as x FROM (select e1 x, e2 + 1 y from pm1.g1) a) AS b", //$NON-NLS-1$ + FakeMetadataFactory.example1Cached(), null, new DefaultCapabilitiesFinder(caps), + new String[] { + "SELECT MAX(g_0.e1) FROM pm1.g1 AS g_0"}, ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ + + TestOptimizer.checkNodeTypes(plan, TestOptimizer.FULL_PUSHDOWN); + } } Modified: trunk/engine/src/test/java/com/metamatrix/query/processor/TestProcessor.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/processor/TestProcessor.java 2010-03-02 23:23:00 UTC (rev 1887) +++ trunk/engine/src/test/java/com/metamatrix/query/processor/TestProcessor.java 2010-03-03 03:29:06 UTC (rev 1888) @@ -6971,7 +6971,26 @@ sampleData1(manager); helpProcess(plan, manager, expected); } + + /** + * A more direct test of 1, where the nested order by is unrelated + */ + @Test public void testSortWithLimit3() { + String sql = "select c from (select pm1.g1.e3 c from pm1.g1 order by pm1.g1.e2 limit 1) x"; //$NON-NLS-1$ + + FakeMetadataFacade metadata = FakeMetadataFactory.example1Cached(); + + ProcessorPlan plan = helpGetPlan(helpParse(sql), metadata, TestOptimizer.getGenericFinder()); + + List[] expected = new List[] { + Arrays.asList(new Object[] { Boolean.FALSE }), + }; + FakeDataManager manager = new FakeDataManager(); + sampleData1(manager); + helpProcess(plan, manager, expected); + } + @Test public void testCountWithHaving() { String sql = "select e1, count(*) from pm1.g1 group by e1 having count(*) > 1"; //$NON-NLS-1$