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

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'}";