[teiid-commits] teiid SVN: r4596 - in branches/7.7.x/engine/src: main/java/org/teiid/query/mapping/xml and 6 other directories.

teiid-commits at lists.jboss.org teiid-commits at lists.jboss.org
Mon Sep 23 16:26:19 EDT 2013


Author: jolee
Date: 2013-09-23 16:26:19 -0400 (Mon, 23 Sep 2013)
New Revision: 4596

Modified:
   branches/7.7.x/engine/src/main/java/org/teiid/query/eval/Evaluator.java
   branches/7.7.x/engine/src/main/java/org/teiid/query/mapping/xml/ResultSetInfo.java
   branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/CriteriaPlanner.java
   branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java
   branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java
   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/processor/TestProcessor.java
   branches/7.7.x/engine/src/test/java/org/teiid/query/processor/eval/TestCriteriaEvaluator.java
   branches/7.7.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java
Log:
TEIID-2669: In subquery with no values returns wrong result

Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/eval/Evaluator.java
===================================================================
--- branches/7.7.x/engine/src/main/java/org/teiid/query/eval/Evaluator.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/main/java/org/teiid/query/eval/Evaluator.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -450,15 +450,18 @@
             throw new ExpressionEvaluationException(e, "ERR.015.006.0015", QueryPlugin.Util.getString("ERR.015.006.0015", criteria)); //$NON-NLS-1$ //$NON-NLS-2$
 		}
 
-		// Shortcut if null
-		if(leftValue == null) {
-            return null;
-        }
         Boolean result = Boolean.FALSE;
 
         ValueIterator valueIter = null;
         if (criteria instanceof SetCriteria) {
         	SetCriteria set = (SetCriteria)criteria;
+       		// Shortcut if null
+    		if(leftValue == null) {
+     			if (!set.getValues().isEmpty()) {
+     				return null;
+     			}
+     			return criteria.isNegated();
+         	}
         	if (set.isAllConstants()) {
         		boolean exists = set.getValues().contains(new Constant(leftValue, criteria.getExpression().getType()));
         		if (!exists) {
@@ -474,6 +477,9 @@
         	ContextReference ref = (ContextReference)criteria;
         	VariableContext vc = getContext(criteria).getVariableContext();
     		ValueIteratorSource vis = (ValueIteratorSource)vc.getGlobalValue(ref.getContextSymbol());
+       		if(leftValue == null) {
+     			return null;
+         	}
     		Set<Object> values;
     		try {
     			values = vis.getCachedSet(ref.getValueExpression());
@@ -497,6 +503,9 @@
         	throw new AssertionError("unknown set criteria type"); //$NON-NLS-1$
         }
         while(valueIter.hasNext()) {
+        	if(leftValue == null) {
+     			return null;
+         	}
             Object possibleValue = valueIter.next();
             Object value = null;
             if(possibleValue instanceof Expression) {
@@ -550,11 +559,6 @@
             throw new ExpressionEvaluationException(e, "ERR.015.006.0015", QueryPlugin.Util.getString("ERR.015.006.0015", criteria)); //$NON-NLS-1$ //$NON-NLS-2$
         }
 
-        // Shortcut if null
-        if(leftValue == null) {
-            return null;
-        }
-
         // Need to be careful to initialize this variable carefully for the case
         // where valueIterator has no values, and the block below is not entered.
         // If there are no rows, and ALL is the predicate quantifier, the result
@@ -573,6 +577,11 @@
 		}
         while(valueIter.hasNext()) {
             Object value = valueIter.next();
+            
+            // Shortcut if null
+            if(leftValue == null) {
+                return null;
+            } 
 
             if(value != null) {
             	int compare = compareValues(leftValue, value);

Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/mapping/xml/ResultSetInfo.java
===================================================================
--- branches/7.7.x/engine/src/main/java/org/teiid/query/mapping/xml/ResultSetInfo.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/main/java/org/teiid/query/mapping/xml/ResultSetInfo.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -65,7 +65,6 @@
     
     private ElementSymbol mappingClassSymbol;
 	private boolean inputSet;
-	private boolean isCritNullDependent;
 
 	//auto-staging related info
 	private String stagingRoot;
@@ -182,14 +181,6 @@
 		this.inputSet = inputSet;
 	}
 	
-	public void setCritNullDependent(boolean isCritNullDependent) {
-		this.isCritNullDependent = isCritNullDependent;
-	}
-	
-	public boolean isCritNullDependent(){
-		return this.isCritNullDependent;
-	}
-	
 	public String getStagingRoot() {
 		return stagingRoot;
 	}

Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/CriteriaPlanner.java
===================================================================
--- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/CriteriaPlanner.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/CriteriaPlanner.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -37,7 +37,6 @@
 import org.teiid.query.mapping.xml.MappingSourceNode;
 import org.teiid.query.mapping.xml.ResultSetInfo;
 import org.teiid.query.metadata.QueryMetadataInterface;
-import org.teiid.query.optimizer.relational.rules.JoinUtil;
 import org.teiid.query.sql.lang.CompareCriteria;
 import org.teiid.query.sql.lang.Criteria;
 import org.teiid.query.sql.symbol.Constant;
@@ -45,7 +44,6 @@
 import org.teiid.query.sql.symbol.Function;
 import org.teiid.query.sql.symbol.GroupSymbol;
 import org.teiid.query.sql.visitor.ElementCollectorVisitor;
-import org.teiid.query.sql.visitor.GroupsUsedByElementsVisitor;
 
 
 public class CriteriaPlanner {
@@ -108,13 +106,8 @@
             //TODO: this can be replaced with method on the source node?
             MappingSourceNode criteriaRs = findRootResultSetNode(context, sourceNodes, criteria);
             
-            Collection<GroupSymbol> groups = GroupsUsedByElementsVisitor.getGroups(conjunct);
-            boolean userCritNullDependent = JoinUtil.isNullDependent(planEnv.getGlobalMetadata(), groups, conjunct);
             ResultSetInfo rs = criteriaRs.getResultSetInfo();
-            if(userCritNullDependent){
-            	rs.setCritNullDependent(true);
-            }
-            
+           
             Criteria convertedCrit = XMLNodeMappingVisitor.convertCriteria(conjunct, planEnv.mappingDoc, planEnv.getGlobalMetadata());
             
             rs.setCriteria(Criteria.combineCriteria(rs.getCriteria(), convertedCrit));

Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java
===================================================================
--- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -310,7 +310,7 @@
             updateSymbolMap(symbolMap, childRsInfo.getResultSetName(), inlineViewName, planEnv.getGlobalMetadata());
             
             // check if the criteria has been raised, if it is then we can update this as a join.
-            if (!rsInfo.isCritNullDependent() && childRsInfo.hasInputSet() && childRsInfo.isCriteriaRaised()) {
+            if (childRsInfo.hasInputSet() && childRsInfo.isCriteriaRaised()) {
                 Query transformationQuery = (Query) command;
                 SubqueryFromClause sfc = (SubqueryFromClause)transformationQuery.getFrom().getClauses().get(0);
                 

Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java
===================================================================
--- branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -976,13 +976,13 @@
 		    }
 		} else if (criteria instanceof SubquerySetCriteria) {
 		    SubquerySetCriteria sub = (SubquerySetCriteria)criteria;
-		    if (rewriteLeftExpression(sub)) {
-		    	return UNKNOWN_CRITERIA;
-		    }
 		    rewriteSubqueryContainer(sub, true);
 		    if (!RelationalNodeUtil.shouldExecute(sub.getCommand(), false, true)) {
-		    	return getSimpliedCriteria(criteria, sub.getExpression(), !sub.isNegated(), true);
+		    	return sub.isNegated()?TRUE_CRITERIA:FALSE_CRITERIA;
 		    }
+		    if (rewriteLeftExpression(sub)) {
+ 		    	addImplicitLimit(sub, 1);
+ 		    }
         } else if (criteria instanceof DependentSetCriteria) {
             criteria = rewriteDependentSetCriteria((DependentSetCriteria)criteria);
         } else if (criteria instanceof ExpressionCriteria) {
@@ -1436,7 +1436,7 @@
         Expression leftExpr = rewriteExpressionDirect(criteria.getLeftExpression());
         
         if (isNull(leftExpr)) {
-            return UNKNOWN_CRITERIA;
+            addImplicitLimit(criteria, 1);
         }
         
         criteria.setLeftExpression(leftExpr);
@@ -1448,7 +1448,12 @@
         rewriteSubqueryContainer(criteria, true);
         
         if (!RelationalNodeUtil.shouldExecute(criteria.getCommand(), false, true)) {
-	    	return getSimpliedCriteria(criteria, criteria.getLeftExpression(), criteria.getPredicateQuantifier()==SubqueryCompareCriteria.ALL, true);
+	       	//TODO: this is not interpretted the same way in all databases
+         	//for example H2 treat both cases as false - however the spec and all major vendors support the following: 
+         	if (criteria.getPredicateQuantifier()==SubqueryCompareCriteria.SOME) {
+         		return FALSE_CRITERIA;
+         	}
+         	return TRUE_CRITERIA;
 	    }
 
         return criteria;
@@ -1730,9 +1735,6 @@
         		return crit; //just return as is
         	}
         } else {
-        	if (newValues.isEmpty()) {
-        		return getSimpliedCriteria(crit, leftExpr, !crit.isNegated(), true);
-        	}
 	        crit.setExpression(leftExpr);
 	        crit.setValues(newValues);
         }
@@ -2022,7 +2024,7 @@
 		
 		criteria.setExpression(rewriteExpressionDirect(criteria.getExpression()));
         
-        if (rewriteLeftExpression(criteria)) {
+        if (rewriteLeftExpression(criteria) && !criteria.getValues().isEmpty()) {
             return UNKNOWN_CRITERIA;
         }
 
@@ -2057,7 +2059,7 @@
         	if (hasNull) {
         		return UNKNOWN_CRITERIA;
         	}
-        	return getSimpliedCriteria(criteria, criteria.getExpression(), !criteria.isNegated(), true);
+        	return criteria.isNegated()?TRUE_CRITERIA:FALSE_CRITERIA;
         }
         
         if(criteria.getExpression() instanceof Function ) {

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-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -2696,7 +2696,7 @@
     	BasicSourceCapabilities caps = getTypicalCapabilities();
     	caps.setFunctionSupport(SourceSystemFunctions.CONVERT, true);
         ProcessorPlan plan = helpPlan("select pm1.g1.e1 from pm1.g1, pm1.g2, pm1.g3 where pm1.g1.e1 = pm1.g2.e1 and pm1.g1.e1 = pm1.g3.e2 and pm1.g3.e2 = pm1.g2.e1", RealMetadataFactory.example1Cached(), //$NON-NLS-1$
-            new String[] { "SELECT g_0.e1 FROM pm1.g1 AS g_0, pm1.g2 AS g_1, pm1.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_0.e1 = convert(g_2.e2, string)) AND (convert(g_2.e2, string) = g_1.e1)" }
+            new String[] { "SELECT g_0.e1 FROM pm1.g1 AS g_0, pm1.g2 AS g_1, pm1.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_0.e1 = g_2.e2) AND (g_2.e2 = g_1.e1)" }
         , new DefaultCapabilitiesFinder(caps), ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$
         checkNodeTypes(plan, FULL_PUSHDOWN);         
     }

Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestProcessor.java
===================================================================
--- branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestProcessor.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestProcessor.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -2784,6 +2784,7 @@
 
         // Create expected results
         List[] expected = new List[] {
+            Arrays.asList(new Object[] { null }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "0" }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "0" }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "1" }), //$NON-NLS-1$
@@ -2982,6 +2983,7 @@
 
         // Create expected results
         List[] expected = new List[]{
+        	Arrays.asList(new Object[] { null }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "0" }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "0" }), //$NON-NLS-1$
             Arrays.asList(new Object[] { "1" }), //$NON-NLS-1$

Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/processor/eval/TestCriteriaEvaluator.java
===================================================================
--- branches/7.7.x/engine/src/test/java/org/teiid/query/processor/eval/TestCriteriaEvaluator.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/test/java/org/teiid/query/processor/eval/TestCriteriaEvaluator.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -412,6 +412,17 @@
         SubqueryCompareCriteria crit = helpGetCompareSubqueryCriteria(SubqueryCompareCriteria.EQ, SubqueryCompareCriteria.ALL);
         helpTestCompareSubqueryCriteria(crit, true, Collections.emptyList()); 
     }
+    
+    /**
+      * The check for empty rows should happen before the check for a null left expression
+      * @throws Exception
+      */
+     @Test public void testCompareSubqueryCriteriaNoRows1() throws Exception {
+         SubqueryCompareCriteria crit = helpGetCompareSubqueryCriteria(SubqueryCompareCriteria.EQ, SubqueryCompareCriteria.ANY);
+         crit.setLeftExpression(new Constant(null));
+         crit.negate();
+         helpTestCompareSubqueryCriteria(crit, true, Collections.emptyList()); 
+     }
 
     /**
      * Special case: if ANY/SOME is specified and the subquery returns no rows,

Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java
===================================================================
--- branches/7.7.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java	2013-09-13 12:58:49 UTC (rev 4595)
+++ branches/7.7.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java	2013-09-23 20:26:19 UTC (rev 4596)
@@ -242,13 +242,17 @@
     @Test public void testRewriteInCriteriaWithNoValues() throws Exception {
     	ElementSymbol e1 = new ElementSymbol("e1");
     	e1.setGroupSymbol(new GroupSymbol("g1"));
-        Criteria crit = new SetCriteria(e1, Collections.EMPTY_LIST); //$NON-NLS-1$
+        SetCriteria crit = new SetCriteria(e1, Collections.EMPTY_LIST); //$NON-NLS-1$
         
         Criteria actual = QueryRewriter.rewriteCriteria(crit, null, null, null);
         
-        IsNullCriteria inc = new IsNullCriteria(e1);
-        inc.setNegated(true);
-        assertEquals(inc, actual);
+        assertEquals(QueryRewriter.FALSE_CRITERIA, actual);
+         
+        crit.setNegated(true);
+        
+        actual = QueryRewriter.rewriteCriteria(crit, null, null, null);
+        
+        assertEquals(QueryRewriter.TRUE_CRITERIA, actual);
     }
         
     @Test public void testRewriteBetweenCriteria1() {
@@ -820,7 +824,7 @@
     
     @Test public void testCompareSubqueryUnknown() {
         helpTestRewriteCommand("SELECT e1 FROM pm1.g1 WHERE null = SOME (SELECT e1 FROM pm1.g2)", //$NON-NLS-1$
-                                "SELECT e1 FROM pm1.g1 WHERE 1 = 0"); //$NON-NLS-1$
+                                "SELECT e1 FROM pm1.g1 WHERE null IN (SELECT e1 FROM pm1.g2 LIMIT 1)"); //$NON-NLS-1$
     }
 
     @Test public void testINClauseSubquery() {
@@ -2439,13 +2443,17 @@
     }
     
     @Test public void testRewriteCritSubqueryFalse1() {
-        helpTestRewriteCriteria("not(pm1.g1.e1 > SOME (select 'a' from pm1.g1 where 1=0))", "pm1.g1.e1 IS NOT NULL"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(pm1.g1.e1 > SOME (select 'a' from pm1.g1 where 1=0))", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$
     }
     
     @Test public void testRewriteCritSubqueryFalse2() {
-        helpTestRewriteCriteria("pm1.g1.e1 < ALL (select 'a' from pm1.g1 where 1=0)", "pm1.g1.e1 IS NOT NULL"); //$NON-NLS-1$ //$NON-NLS-2$
+         helpTestRewriteCriteria("pm1.g1.e1 < ALL (select 'a' from pm1.g1 where 1=0)", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$
     }
     
+    @Test public void testRewriteCritSubqueryFalse3() {
+        helpTestRewriteCriteria("pm1.g1.e1 not in (select 'a' from pm1.g1 where 1=0)", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$
+    }
+    
 	@Test public void testUDFParse() throws Exception {     
         QueryMetadataInterface metadata = RealMetadataFactory.createTransformationMetadata(RealMetadataFactory.example1Cached().getMetadataStore(), "example1", new FunctionTree("foo", new FakeFunctionMetadataSource()));
 		String sql = "parsedate_(pm1.g1.e1) = {d'2001-01-01'}";



More information about the teiid-commits mailing list