Author: jolee Date: 2013-08-22 10:43:08 -0400 (Thu, 22 Aug 2013) New Revision: 4592 Modified: branches/7.7.x/build/kits/jboss-container/teiid-releasenotes.html branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCleanCriteria.java branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCopyCriteria.java branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseNull.java branches/7.7.x/engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java Log: BZ975033: EDS_5.3.1_3_2013 + Teiid-2633: Criteria is assumed needed when no other predicates determined during RuleRaiseAccess Modified: branches/7.7.x/build/kits/jboss-container/teiid-releasenotes.html =================================================================== --- branches/7.7.x/build/kits/jboss-container/teiid-releasenotes.html 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/build/kits/jboss-container/teiid-releasenotes.html 2013-08-22 14:43:08 UTC (rev 4592) @@ -258,6 +258,8 @@ </li> <li>[<a href='https://issues.jboss.org/browse/TEIID-2611'>TEIID-2611&l...>] - Client forwards compatibility issue </li> +<li>[<a href='https://issues.jboss.org/browse/TEIID-2633'>TEIID-2633&l...>] - Criteria is assumed needed when no other predicates determined during RuleRaiseAccess +</li> </ul> <h4>From 7.7.7</h4> Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2013-08-22 14:43:08 UTC (rev 4592) @@ -469,6 +469,12 @@ rules.push(RuleConstants.PUSH_SELECT_CRITERIA); rules.push(RuleConstants.PLAN_JOINS); } + + if(hints.hasJoin) { + rules.push(RuleConstants.CLEAN_CRITERIA); + rules.push(RuleConstants.COPY_CRITERIA); + } + rules.push(RuleConstants.RAISE_ACCESS); if (hints.hasSetQuery) { rules.push(RuleConstants.PLAN_UNIONS); @@ -479,7 +485,6 @@ rules.push(RuleConstants.CLEAN_CRITERIA); } if(hints.hasJoin) { - rules.push(RuleConstants.COPY_CRITERIA); rules.push(RuleConstants.PUSH_NON_JOIN_CRITERIA); } if(hints.hasVirtualGroups) { Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCleanCriteria.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCleanCriteria.java 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCleanCriteria.java 2013-08-22 14:43:08 UTC (rev 4592) @@ -54,7 +54,7 @@ boolean pushRaiseNull = false; - pushRaiseNull = clean(plan); + pushRaiseNull = clean(plan, !rules.contains(RuleConstants.COPY_CRITERIA)); if (pushRaiseNull) { rules.push(RuleConstants.RAISE_NULL); @@ -63,24 +63,25 @@ return plan; } - private boolean clean(PlanNode plan) + private boolean clean(PlanNode plan, boolean removeAllPhantom) throws TeiidComponentException { boolean pushRaiseNull = false; plan.setProperty(Info.OUTPUT_COLS, null); for (PlanNode node : plan.getChildren()) { - pushRaiseNull |= clean(node); + pushRaiseNull |= clean(node, removeAllPhantom); } if (plan.getType() == NodeConstants.Types.SELECT) { - pushRaiseNull |= cleanCriteria(plan); + pushRaiseNull |= cleanCriteria(plan, removeAllPhantom); } return pushRaiseNull; } - boolean cleanCriteria(PlanNode critNode) throws TeiidComponentException { - if (critNode.hasBooleanProperty(NodeConstants.Info.IS_PHANTOM)) { - NodeEditor.removeChildNode(critNode.getParent(), critNode); - return false; - } + boolean cleanCriteria(PlanNode critNode, boolean removeAllPhantom) throws TeiidComponentException { + if (critNode.hasBooleanProperty(NodeConstants.Info.IS_PHANTOM) + && (removeAllPhantom || critNode.hasBooleanProperty(Info.IS_COPIED))) { + NodeEditor.removeChildNode(critNode.getParent(), critNode); + return false; + } //TODO: remove dependent set criteria that has not been meaningfully pushed from its parent join Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCopyCriteria.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCopyCriteria.java 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCopyCriteria.java 2013-08-22 14:43:08 UTC (rev 4592) @@ -96,9 +96,10 @@ return plan; } - if (tryToCopy(plan, new Set[2], metadata)) { + if (tryToCopy(plan, new Set[2], metadata, false)) { //Push any newly created criteria nodes and try to copy them afterwards rules.push(RuleConstants.COPY_CRITERIA); + rules.push(RuleConstants.RAISE_ACCESS); rules.push(RuleConstants.PUSH_NON_JOIN_CRITERIA); } @@ -127,7 +128,9 @@ List<Criteria> joinCriteria, Set<Criteria> combinedCriteria, boolean checkForGroupReduction, - QueryMetadataInterface metadata) { + QueryMetadataInterface metadata, + boolean underAccess) { + int startGroups = GroupsUsedByElementsVisitor.getGroups(crit).size(); Criteria tgtCrit = (Criteria) crit.clone(); @@ -153,8 +156,28 @@ return false; } + boolean isNew = combinedCriteria.add(tgtCrit); + + if (underAccess) { + boolean use = false; + if (isNew && !checkForGroupReduction) { + if (endGroups == 1) { + use = true; + } else if (tgtCrit instanceof CompareCriteria) { + CompareCriteria cc = (CompareCriteria)tgtCrit; + int leftGroups = GroupsUsedByElementsVisitor.getGroups(cc.getLeftExpression()).size(); + if (leftGroups == endGroups || leftGroups == 0) { + use = true; + } + } + } + if (!use) { + return false; + } + } + //if this is unique or it a duplicate but reduced a current join conjunct, return true - if (combinedCriteria.add(tgtCrit)) { + if (isNew) { joinCriteria.add(tgtCrit); return true; } else if (checkForGroupReduction) { @@ -172,7 +195,7 @@ * @param node * @return true if criteria has been created */ - private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMetadataInterface metadata) { + private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMetadataInterface metadata, boolean underAccess) { boolean changedTree = false; if (node == null) { @@ -184,14 +207,15 @@ JoinType jt = (JoinType)node.getProperty(NodeConstants.Info.JOIN_TYPE); if (jt == JoinType.JOIN_FULL_OUTER) { - return visitChildern(node, criteriaInfo, changedTree, metadata); + return visitChildern(node, criteriaInfo, changedTree, metadata, underAccess); } Set<Criteria>[] leftChildCriteria = new Set[2]; Set<Criteria>[] rightChildCriteria = new Set[2]; - changedTree |= tryToCopy(node.getFirstChild(), leftChildCriteria, metadata); - changedTree |= tryToCopy(node.getLastChild(), rightChildCriteria, metadata); + changedTree |= tryToCopy(node.getFirstChild(), leftChildCriteria, metadata, underAccess); + changedTree |= tryToCopy(node.getLastChild(), rightChildCriteria, metadata, underAccess); + List<Criteria> joinCrits = (List<Criteria>) node.getProperty(NodeConstants.Info.JOIN_CRITERIA); Set<Criteria> combinedCriteria = null; @@ -227,11 +251,11 @@ if (!toCopy.isEmpty()) { - changedTree |= createCriteria(false, toCopy, combinedCriteria, srcToTgt, newJoinCrits, metadata); + changedTree |= createCriteria(false, toCopy, combinedCriteria, srcToTgt, newJoinCrits, metadata, underAccess); srcToTgt = buildElementMap(allCriteria, null, null, metadata); - changedTree |= createCriteria(true, joinCrits, combinedCriteria, srcToTgt, newJoinCrits, metadata); + changedTree |= createCriteria(true, joinCrits, combinedCriteria, srcToTgt, newJoinCrits, metadata, underAccess); } joinCrits.addAll(newJoinCrits); @@ -254,7 +278,7 @@ return changedTree; } - changedTree = visitChildern(node, criteriaInfo, changedTree, metadata); + changedTree = visitChildern(node, criteriaInfo, changedTree, metadata, underAccess); //visit select nodes on the way back up switch (node.getType()) { @@ -292,7 +316,7 @@ Set<Criteria> combinedCriteria, Map<Expression, Expression> srcToTgt, List<Criteria> newJoinCrits, - QueryMetadataInterface metadata) { + QueryMetadataInterface metadata, boolean underAccess) { boolean changedTree = false; if (srcToTgt.size() == 0) { return changedTree; @@ -301,7 +325,7 @@ while (i.hasNext()) { Criteria crit = i.next(); - if (copyCriteria(crit, srcToTgt, newJoinCrits, combinedCriteria, copyingJoinCriteria, metadata)) { + if (copyCriteria(crit, srcToTgt, newJoinCrits, combinedCriteria, copyingJoinCriteria, metadata, underAccess)) { changedTree = true; if (!copyingJoinCriteria) { crit = newJoinCrits.get(newJoinCrits.size() - 1); @@ -342,12 +366,13 @@ private boolean visitChildern(PlanNode node, Set<Criteria>[] criteriaInfo, boolean changedTree, - QueryMetadataInterface metadata) { + QueryMetadataInterface metadata, boolean underAccess) { if (node.getChildCount() > 0) { + underAccess |= node.getType() == NodeConstants.Types.ACCESS; List<PlanNode> children = node.getChildren(); for (int i = 0; i < children.size(); i++) { PlanNode childNode = children.get(i); - changedTree |= tryToCopy(childNode, i==0?criteriaInfo:new Set[2], metadata); + changedTree |= tryToCopy(childNode, i==0?criteriaInfo:new Set[2], metadata, underAccess); } } return changedTree; Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseNull.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseNull.java 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseNull.java 2013-08-22 14:43:08 UTC (rev 4592) @@ -36,6 +36,7 @@ import org.teiid.query.optimizer.relational.OptimizerRule; import org.teiid.query.optimizer.relational.RuleStack; import org.teiid.query.optimizer.relational.plantree.NodeConstants; +import org.teiid.query.optimizer.relational.plantree.NodeConstants.Info; import org.teiid.query.optimizer.relational.plantree.NodeEditor; import org.teiid.query.optimizer.relational.plantree.NodeFactory; import org.teiid.query.optimizer.relational.plantree.PlanNode; @@ -220,7 +221,7 @@ case NodeConstants.Types.PROJECT: { // check for project into - PlanNode upperProject = NodeEditor.findParent(parentNode.getParent(), NodeConstants.Types.PROJECT); + PlanNode upperProject = NodeEditor.findParent(parentNode.getParent(), NodeConstants.Types.PROJECT, NodeConstants.Types.SOURCE); if (upperProject == null || upperProject.getProperty(NodeConstants.Info.INTO_GROUP) == null) { @@ -228,6 +229,14 @@ } break; } + case NodeConstants.Types.SOURCE: + { + PlanNode upperProject = parentNode.getParent(); + if (upperProject != null && upperProject.getType() == NodeConstants.Types.PROJECT && upperProject.hasProperty(Info.INTO_GROUP)) { + break; //an insert plan + } + return raiseNullNode(rootNode, parentNode, nullNode, nodes); + } default: { return raiseNullNode(rootNode, parentNode, nullNode, nodes); Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java =================================================================== --- branches/7.7.x/engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java 2013-08-15 06:35:56 UTC (rev 4591) +++ branches/7.7.x/engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java 2013-08-22 14:43:08 UTC (rev 4592) @@ -2649,10 +2649,9 @@ */ @Test public void testCopyCriteriaWithOuterJoin5_defect10050(){ - ProcessorPlan plan = helpPlan( - "select pm2.g1.e1, pm2.g2.e1, pm2.g3.e1 from ( (pm2.g1 right outer join pm2.g2 on pm2.g1.e1=pm2.g2.e1) right outer join pm2.g3 on pm2.g2.e1=pm2.g3.e1) where pm2.g3.e1 = 'a'", example1(), //$NON-NLS-1$ - new String[] { "SELECT g_2.e1, g_1.e1, g_0.e1 FROM pm2.g3 AS g_0 LEFT OUTER JOIN (pm2.g2 AS g_1 LEFT OUTER JOIN pm2.g1 AS g_2 ON g_2.e1 = g_1.e1 AND g_2.e1 = 'a') ON g_1.e1 = g_0.e1 AND g_1.e1 = 'a' WHERE g_0.e1 = 'a'" }); //$NON-NLS-1$ - checkNodeTypes(plan, FULL_PUSHDOWN); + helpPlan("select pm2.g1.e1, pm1.g2.e1, pm2.g3.e1 from ( (pm2.g1 right outer join pm1.g2 on pm2.g1.e1=pm1.g2.e1) right outer join pm2.g3 on pm1.g2.e1=pm2.g3.e1) where pm2.g3.e1 = 'a'", example1(), //$NON-NLS-1$ + new String[] { "SELECT g_0.e1 FROM pm1.g2 AS g_0 WHERE g_0.e1 = 'a'", "SELECT g_0.e1 FROM pm2.g1 AS g_0 WHERE g_0.e1 = 'a'", "SELECT g_0.e1 FROM pm2.g3 AS g_0 WHERE g_0.e1 = 'a'" }); //$NON-NLS-1$ + } /** @@ -2673,8 +2672,20 @@ ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1 right outer join pm2.g2 on pm2.g2.e1=pm2.g1.e1 where pm2.g2.e1 IN ('a', 'b')", example1(), //$NON-NLS-1$ new String[] { "SELECT pm2.g1.e1, pm2.g2.e1 FROM pm2.g2 LEFT OUTER JOIN pm2.g1 ON pm2.g2.e1 = pm2.g1.e1 AND pm2.g1.e1 IN ('a', 'b') WHERE pm2.g2.e1 IN ('a', 'b')" }); //$NON-NLS-1$ checkNodeTypes(plan, FULL_PUSHDOWN); - } + } + + @Test public void testCopyCriteriaWithTransitivePushdown(){ + ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1, pm2.g2, pm2.g3 where pm2.g1.e1 = pm2.g2.e1 and pm2.g2.e1 = pm2.g3.e1", example1(), //$NON-NLS-1$ + new String[] { "SELECT g_0.e1, g_1.e1 FROM pm2.g1 AS g_0, pm2.g2 AS g_1, pm2.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_1.e1 = g_2.e1)" }); //$NON-NLS-1$ + checkNodeTypes(plan, FULL_PUSHDOWN); + } + @Test public void testCopyCriteriaWithTransitivePushdown1(){ + ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1, pm2.g2, pm2.g3 where pm2.g1.e1 = pm2.g2.e1 and pm2.g2.e1 = pm2.g3.e1 and pm2.g1.e1 = 'a'", example1(), //$NON-NLS-1$ + new String[] { "SELECT g_0.e1, g_1.e1 FROM pm2.g1 AS g_0, pm2.g2 AS g_1, pm2.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_1.e1 = g_2.e1) AND (g_0.e1 = 'a') AND (g_1.e1 = 'a') AND (g_2.e1 = 'a')" }); //$NON-NLS-1$ + checkNodeTypes(plan, FULL_PUSHDOWN); + } + @Test public void testCleanCriteria(){ ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1, pm2.g2 where pm2.g1.e1=pm2.g2.e1 and pm2.g1.e2 IN (1, 2)", example1(), //$NON-NLS-1$