Author: shawkins Date: 2012-10-05 09:39:14 -0400 (Fri, 05 Oct 2012) New Revision: 4516 Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/capabilities/BasicSourceCapabilities.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCollapseSource.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleImplementJoinStrategy.java trunk/engine/src/test/java/org/teiid/query/optimizer/TestJoinOptimization.java trunk/test-integration/common/src/test/java/org/teiid/dqp/internal/process/TestTPCR.java Log: TEIID-2245 limiting when nulls first/last is pushed down Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/capabilities/BasicSourceCapabilities.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/capabilities/BasicSourceCapabilities.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/capabilities/BasicSourceCapabilities.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -29,7 +29,6 @@ import org.teiid.translator.ExecutionFactory; import org.teiid.translator.ExecutionFactory.Format; -import org.teiid.translator.ExecutionFactory.NullOrder; /** */ @@ -46,8 +45,6 @@ * Construct a basic capabilities object. */ public BasicSourceCapabilities() { - //for test compatibility we'll assume low, this does get replaced by a proper value at runtime - setSourceProperty(Capability.QUERY_ORDERBY_DEFAULT_NULL_ORDER, NullOrder.LOW); } public boolean supportsCapability(Capability capability) { Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -501,6 +501,7 @@ } else { JoinNode joinAsSet = new JoinNode(getID()); joinAsSet.setJoinStrategy(new MergeJoinStrategy(SortOption.SORT_DISTINCT, SortOption.SORT_DISTINCT, true)); + //If we push these sorts, we will have to enforce null order, since nulls are equal here List leftExpressions = (List) node.getFirstChild().getProperty(NodeConstants.Info.OUTPUT_COLS); List rightExpressions = (List) node.getLastChild().getProperty(NodeConstants.Info.OUTPUT_COLS); joinAsSet.setJoinType(setOp == Operation.EXCEPT ? JoinType.JOIN_ANTI_SEMI : JoinType.JOIN_SEMI); Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCollapseSource.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCollapseSource.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCollapseSource.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -490,6 +490,7 @@ } private void processOrderBy(PlanNode node, QueryCommand query, Object modelID, CommandContext context, CapabilitiesFinder capFinder) throws QueryMetadataException, TeiidComponentException { + boolean userOrdering = NodeEditor.findParent(node, NodeConstants.Types.JOIN|NodeConstants.Types.SOURCE) == null; OrderBy orderBy = (OrderBy)node.getProperty(NodeConstants.Info.SORT_ORDER); query.setOrderBy(orderBy); if (query instanceof Query) { @@ -506,7 +507,7 @@ if (!supportsNullOrdering) { item.setNullOrdering(null); } - } else if (supportsNullOrdering && defaultNullOrder != NullOrder.LOW && context.getOptions().isPushdownDefaultNullOrder()) { + } else if (userOrdering && supportsNullOrdering && defaultNullOrder != NullOrder.LOW && context.getOptions().isPushdownDefaultNullOrder()) { //try to match the expected default of low if (item.isAscending()) { if (defaultNullOrder != NullOrder.FIRST) { Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleImplementJoinStrategy.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleImplementJoinStrategy.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleImplementJoinStrategy.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -34,11 +34,9 @@ import org.teiid.api.exception.query.QueryMetadataException; import org.teiid.api.exception.query.QueryPlannerException; import org.teiid.core.TeiidComponentException; -import org.teiid.language.SortSpecification.NullOrdering; import org.teiid.query.analysis.AnalysisRecord; import org.teiid.query.metadata.QueryMetadataInterface; import org.teiid.query.optimizer.capabilities.CapabilitiesFinder; -import org.teiid.query.optimizer.capabilities.SourceCapabilities.Capability; import org.teiid.query.optimizer.relational.OptimizerRule; import org.teiid.query.optimizer.relational.RuleStack; import org.teiid.query.optimizer.relational.plantree.NodeConstants; @@ -50,19 +48,15 @@ import org.teiid.query.processor.relational.JoinNode.JoinStrategyType; import org.teiid.query.processor.relational.MergeJoinStrategy.SortOption; import org.teiid.query.sql.lang.CompareCriteria; -import org.teiid.query.sql.lang.CompoundCriteria; import org.teiid.query.sql.lang.Criteria; -import org.teiid.query.sql.lang.IsNullCriteria; import org.teiid.query.sql.lang.JoinType; import org.teiid.query.sql.lang.OrderBy; -import org.teiid.query.sql.lang.OrderByItem; import org.teiid.query.sql.symbol.ElementSymbol; import org.teiid.query.sql.symbol.Expression; import org.teiid.query.sql.symbol.GroupSymbol; import org.teiid.query.sql.util.SymbolMap; import org.teiid.query.sql.visitor.GroupsUsedByElementsVisitor; import org.teiid.query.util.CommandContext; -import org.teiid.translator.ExecutionFactory.NullOrder; /** @@ -255,40 +249,12 @@ joinNode.setProperty(joinNode.getFirstChild() == childNode ? NodeConstants.Info.IS_LEFT_DISTINCT : NodeConstants.Info.IS_RIGHT_DISTINCT, true); } if (attemptPush && RuleRaiseAccess.canRaiseOverSort(sourceNode, metadata, capFinder, sortNode, null, false)) { - boolean push = true; - Object model = RuleRaiseAccess.getModelIDFromAccess(sourceNode, metadata); - NullOrder order = CapabilitiesUtil.getDefaultNullOrder(model, metadata, capFinder); - if (order != NullOrder.LOW && order != NullOrder.FIRST) { - //TODO: check the metadata and query structure to see if nulls are possible - JoinType joinType = (JoinType) joinNode.getProperty(NodeConstants.Info.JOIN_TYPE); - if (joinType == JoinType.JOIN_INNER || joinType == JoinType.JOIN_SEMI || ((joinType == JoinType.JOIN_LEFT_OUTER || joinType == JoinType.JOIN_ANTI_SEMI) && joinNode.getLastChild() == childNode)) { - //just filter nulls on the teiid side - CompoundCriteria crit = new CompoundCriteria(); - for (Expression expression : orderSymbols) { - IsNullCriteria inc = new IsNullCriteria((Expression) expression.clone()); - inc.setNegated(true); - crit.addCriteria(inc); - } - PlanNode critNode = NodeFactory.getNewNode(NodeConstants.Types.SELECT); - critNode.setProperty(NodeConstants.Info.OUTPUT_COLS, new ArrayList<Expression>(outputSymbols)); - critNode.setProperty(NodeConstants.Info.SELECT_CRITERIA, crit); - sourceNode.addAsParent(critNode); - } else if (CapabilitiesUtil.supports(Capability.QUERY_ORDERBY_NULL_ORDERING, model, metadata, capFinder)) { - for (OrderByItem item : ((OrderBy)sortNode.getProperty(NodeConstants.Info.SORT_ORDER)).getOrderByItems()) { - item.setNullOrdering(NullOrdering.FIRST); - } - } else { - push = false; - } + sourceNode.getFirstChild().addAsParent(sortNode); + + if (needsCorrection) { + correctOutputElements(joinNode, outputSymbols, sortNode); } - if (push) { - sourceNode.getFirstChild().addAsParent(sortNode); - - if (needsCorrection) { - correctOutputElements(joinNode, outputSymbols, sortNode); - } - return true; - } + return true; } } else if (sourceNode.getType() == NodeConstants.Types.GROUP) { sourceNode.addAsParent(sortNode); Modified: trunk/engine/src/test/java/org/teiid/query/optimizer/TestJoinOptimization.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/optimizer/TestJoinOptimization.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/engine/src/test/java/org/teiid/query/optimizer/TestJoinOptimization.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -52,7 +52,6 @@ import org.teiid.query.sql.symbol.GroupSymbol; import org.teiid.query.unittest.RealMetadataFactory; import org.teiid.translator.SourceSystemFunctions; -import org.teiid.translator.ExecutionFactory.NullOrder; @SuppressWarnings("nls") public class TestJoinOptimization { @@ -1049,63 +1048,4 @@ } - @Test public void testJoinNullHandling() throws Exception { - FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder(); - BasicSourceCapabilities bqt1 = new BasicSourceCapabilities(); - bqt1.setCapabilitySupport(Capability.QUERY_ORDERBY, true); - bqt1.setSourceProperty(Capability.QUERY_ORDERBY_DEFAULT_NULL_ORDER, NullOrder.HIGH); - bqt1.setCapabilitySupport(Capability.QUERY_ORDERBY_NULL_ORDERING, true); - - BasicSourceCapabilities bqt2 = new BasicSourceCapabilities(); - bqt2.setCapabilitySupport(Capability.QUERY_ORDERBY, true); - bqt2.setSourceProperty(Capability.QUERY_ORDERBY_DEFAULT_NULL_ORDER, NullOrder.HIGH); - - capFinder.addCapabilities("BQT1", bqt1); //$NON-NLS-1$ - capFinder.addCapabilities("BQT2", bqt2); //$NON-NLS-1$ - - String sql = "select bqt1.smalla.intkey, bqt2.smalla.intkey from bqt1.smalla full outer join bqt2.smalla on (bqt1.smalla.intkey = bqt2.smalla.intkey)"; //$NON-NLS-1$ - - // can't push order by to bqt2 as there is no compensating action available. - ProcessorPlan plan = TestOptimizer.helpPlan(sql, RealMetadataFactory.exampleBQTCached(), new String[] {"SELECT BQT2.SmallA.IntKey FROM BQT2.SmallA", "SELECT BQT1.SmallA.IntKey FROM BQT1.SmallA ORDER BY BQT1.SmallA.IntKey NULLS FIRST"}, capFinder, 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 - 0, // Grouping - 0, // Join - 1, // MergeJoin - 0, // Null - 0, // PlanExecution - 1, // Project - 0, // Select - 0, // Sort - 0 // UnionAll - }); - - sql = "select bqt1.smalla.intkey, bqt2.smalla.intkey from bqt1.smalla left outer join bqt2.smalla on (bqt1.smalla.intkey = bqt2.smalla.intkey)"; //$NON-NLS-1$ - - // can push order by to bqt2 by adding a null filter - plan = TestOptimizer.helpPlan(sql, RealMetadataFactory.exampleBQTCached(), new String[] {"SELECT BQT2.SmallA.IntKey FROM BQT2.SmallA ORDER BY BQT2.SmallA.IntKey", "SELECT BQT1.SmallA.IntKey FROM BQT1.SmallA ORDER BY BQT1.SmallA.IntKey NULLS FIRST"}, capFinder, 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 - 0, // Grouping - 0, // Join - 1, // MergeJoin - 0, // Null - 0, // PlanExecution - 1, // Project - 1, // Select - 0, // Sort - 0 // UnionAll - }); - } - } Modified: trunk/test-integration/common/src/test/java/org/teiid/dqp/internal/process/TestTPCR.java =================================================================== --- trunk/test-integration/common/src/test/java/org/teiid/dqp/internal/process/TestTPCR.java 2012-10-05 12:36:44 UTC (rev 4515) +++ trunk/test-integration/common/src/test/java/org/teiid/dqp/internal/process/TestTPCR.java 2012-10-05 13:39:14 UTC (rev 4516) @@ -121,7 +121,7 @@ new List<?>[] { Arrays.asList(new Object[] { new Long(5), "Bill", "101 Fake St.", "392839283", "21.12" } ), //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ Arrays.asList(new Object[] { new Long(6), "Stu", "102 Fake St.", "385729385", "51.50" } )}; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ - dataMgr.addData("SELECT g_0.C_CUSTKEY AS c_0, g_0.C_NAME AS c_1, g_0.C_ADDRESS AS c_2, g_0.C_PHONE AS c_3, g_0.C_ACCTBAL AS c_4 FROM TPCR_Ora.CUSTOMER AS g_0 WHERE g_0.C_ACCTBAL > 50 ORDER BY c_0 NULLS FIRST", //$NON-NLS-1$ + dataMgr.addData("SELECT g_0.C_CUSTKEY AS c_0, g_0.C_NAME AS c_1, g_0.C_ADDRESS AS c_2, g_0.C_PHONE AS c_3, g_0.C_ACCTBAL AS c_4 FROM TPCR_Ora.CUSTOMER AS g_0 WHERE g_0.C_ACCTBAL > 50 ORDER BY c_0", //$NON-NLS-1$ oracleExpected); List<?>[] sqlServerExpected = @@ -161,7 +161,7 @@ new List<?>[] { Arrays.asList(new Object[] { new Long(5), "Bill", "101 Fake St.", "392839283", "51.12" } ), //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ Arrays.asList(new Object[] { new Long(6), "Stu", "102 Fake St.", "385729385", "51.50" } )}; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ - dataMgr.addData("SELECT g_0.C_CUSTKEY AS c_0, g_0.C_NAME AS c_1, g_0.C_ADDRESS AS c_2, g_0.C_PHONE AS c_3, g_0.C_ACCTBAL AS c_4 FROM TPCR_Ora.CUSTOMER AS g_0 WHERE g_0.C_ACCTBAL > 50 ORDER BY c_0 NULLS FIRST", //$NON-NLS-1$ + dataMgr.addData("SELECT g_0.C_CUSTKEY AS c_0, g_0.C_NAME AS c_1, g_0.C_ADDRESS AS c_2, g_0.C_PHONE AS c_3, g_0.C_ACCTBAL AS c_4 FROM TPCR_Ora.CUSTOMER AS g_0 WHERE g_0.C_ACCTBAL > 50 ORDER BY c_0", //$NON-NLS-1$ oracleExpected); List<?>[] sqlServerExpected =