TEIID-3490 fix for inappropriate with pushdown

Conflicts: engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java
teiid · Jul 1, 2015 · 049e866 · 049e866
1 parent 7816560
commit 049e866
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 5 deletions.
diff --git a/build/kits/jboss-as7/docs/teiid/teiid-releasenotes.html b/build/kits/jboss-as7/docs/teiid/teiid-releasenotes.html
@@ -315,6 +315,8 @@ <h4>from ${project.version}</h4>
 </li>
 <li>[<a href='https://issues.jboss.org/browse/TEIID-3470'>TEIID-3470</a>] -  NPE in ConnectorWorkItem when using source hints
 </li>
+<li>[<a href='https://issues.jboss.org/browse/TEIID-3490'>TEIID-3490</a>] -  WITH [TABLENAME] AS clause being sent to source query
+</li>
 </ul>
 
 <h4>from 8.7.3</h4>

diff --git a/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java b/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java
@@ -288,7 +288,7 @@ private static void assignWithClause(RelationalNode node, Map<String, WithQueryC
             Command command = accessNode.getCommand();
             if (command instanceof QueryCommand) {
             	groups.clear();
-            	GroupCollectorVisitor.getGroupsIgnoreInlineViews(command, groups);
+            	GroupCollectorVisitor.getGroupsIgnoreInlineViewsAndEvaluatableSubqueries(command, groups); 
             	List<WithQueryCommand> with = new ArrayList<WithQueryCommand>();
             	for (GroupSymbol groupSymbol : groups) {
             		WithQueryCommand clause = pushdownWith.get(groupSymbol.getNonCorrelationName());

diff --git a/engine/src/main/java/org/teiid/query/sql/navigator/PreOrPostOrderNavigator.java b/engine/src/main/java/org/teiid/query/sql/navigator/PreOrPostOrderNavigator.java
@@ -42,6 +42,7 @@ public class PreOrPostOrderNavigator extends AbstractNavigator {
 
     private boolean order;
     private boolean deep;
+	private boolean skipEvaluatable;
 
     public PreOrPostOrderNavigator(LanguageVisitor visitor, boolean order, boolean deep) {
         super(visitor);
@@ -171,7 +172,7 @@ public void visit(ElementSymbol obj) {
     }
     public void visit(ExistsCriteria obj) {
         preVisitVisitor(obj);
-        if (deep) {
+        if (deep && (!obj.shouldEvaluate() || !skipEvaluatable)) {
         	visitNode(obj.getCommand());
         }
         postVisitVisitor(obj);        
@@ -322,7 +323,7 @@ public void visit(Reference obj) {
     }
     public void visit(ScalarSubquery obj) {
         preVisitVisitor(obj);
-        if (deep) {
+        if (deep && (!obj.shouldEvaluate() || !skipEvaluatable)) {
         	visitNode(obj.getCommand());
         }
         postVisitVisitor(obj);
@@ -668,5 +669,9 @@ public static void doVisit(LanguageObject object, LanguageVisitor visitor, boole
         PreOrPostOrderNavigator nav = new PreOrPostOrderNavigator(visitor, order, deep);
         object.acceptVisitor(nav);
     }
+
+    public void setSkipEvaluatable(boolean skipEvaluatable) {
+		this.skipEvaluatable = skipEvaluatable;
+	}
 
 }
diff --git a/engine/src/main/java/org/teiid/query/sql/visitor/GroupCollectorVisitor.java b/engine/src/main/java/org/teiid/query/sql/visitor/GroupCollectorVisitor.java
@@ -33,6 +33,7 @@
 import org.teiid.query.sql.lang.StoredProcedure;
 import org.teiid.query.sql.lang.SubqueryFromClause;
 import org.teiid.query.sql.navigator.DeepPreOrderNavigator;
+import org.teiid.query.sql.navigator.PreOrPostOrderNavigator;
 import org.teiid.query.sql.navigator.PreOrderNavigator;
 import org.teiid.query.sql.symbol.GroupSymbol;
 
@@ -162,10 +163,12 @@ public static Collection<GroupSymbol> getGroups(LanguageObject obj, boolean remo
      * @param obj Language object
      * @param elements Collection to collect groups in
      */
-    public static void getGroupsIgnoreInlineViews(LanguageObject obj, Collection<GroupSymbol> groups) {
+    public static void getGroupsIgnoreInlineViewsAndEvaluatableSubqueries(LanguageObject obj, Collection<GroupSymbol> groups) {
         GroupCollectorVisitor visitor = new GroupCollectorVisitor(groups);
         visitor.setIgnoreInlineViewGroups(true);
-        DeepPreOrderNavigator.doVisit(obj, visitor);  
+        PreOrPostOrderNavigator nav = new PreOrPostOrderNavigator(visitor, PreOrPostOrderNavigator.PRE_ORDER, true);
+        nav.setSkipEvaluatable(true);
+        obj.acceptVisitor(nav);  
 
         if(visitor.getInlineViewGroups() != null) {
             groups.removeAll(visitor.getInlineViewGroups());

diff --git a/engine/src/test/java/org/teiid/query/processor/TestWithClauseProcessing.java b/engine/src/test/java/org/teiid/query/processor/TestWithClauseProcessing.java
@@ -16,6 +16,7 @@
 import org.teiid.query.optimizer.TestOptimizer;
 import org.teiid.query.optimizer.TestOptimizer.ComparisonMode;
 import org.teiid.query.optimizer.capabilities.BasicSourceCapabilities;
+import org.teiid.query.optimizer.capabilities.CapabilitiesFinder;
 import org.teiid.query.optimizer.capabilities.DefaultCapabilitiesFinder;
 import org.teiid.query.optimizer.capabilities.FakeCapabilitiesFinder;
 import org.teiid.query.optimizer.capabilities.SourceCapabilities.Capability;
@@ -348,4 +349,13 @@ public TupleSource registerRequest(CommandContext context,
 	    helpProcess(plan, cc, dataManager, expected);
 	}
 
+	@Test public void testWithAndUncorrelatedSubquery() throws TeiidComponentException, TeiidProcessingException {
+	    String sql = "WITH t(n) AS ( select e1 from pm2.g1 ) SELECT n FROM t as t1, pm1.g1 where e1 = (select n from t)"; //$NON-NLS-1$
+
+	    BasicSourceCapabilities bsc = TestOptimizer.getTypicalCapabilities();
+	    bsc.setCapabilitySupport(Capability.COMMON_TABLE_EXPRESSIONS, true);
+	    CapabilitiesFinder capFinder = new DefaultCapabilitiesFinder(bsc);
+	    TestOptimizer.helpPlan(sql, RealMetadataFactory.example1Cached(), new String[] {"SELECT 1 FROM pm1.g1 AS g_0 WHERE g_0.e1 = (WITH t (n) AS (SELECT g_0.e1 FROM pm2.g1 AS g_0) SELECT g_0.n FROM t AS g_0)", "WITH t (n) AS (SELECT g_0.e1 FROM pm2.g1 AS g_0) SELECT g_0.n FROM t AS g_0"}, capFinder, ComparisonMode.EXACT_COMMAND_STRING);
+	}
+
 }