teiid SVN: r4596 - in branches/7.7.x/engine/src: main/java/org/teiid/query/mapping/xml and 6 other directories.
by teiid-commits＠lists.jboss.org
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'}";
