Author: shawkins Date: 2009-08-20 09:14:46 -0400 (Thu, 20 Aug 2009) New Revision: 1266 Modified: trunk/connectors/connector-ldap/src/main/java/com/metamatrix/connector/ldap/IQueryToLdapSearchParser.java trunk/connectors/connector-ldap/src/test/java/com/metamatrix/connector/ldap/TestIQueryToLdapSearchParser.java trunk/connectors/connector-salesforce/src/test/java/com/metamatrix/connector/salesforce/execution/visitors/TestVisitors.java trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractCompareCriteria.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractSetCriteria.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/CompareCriteria.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/IsNullCriteria.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/MatchCriteria.java trunk/engine/src/main/java/com/metamatrix/query/sql/lang/PredicateCriteria.java trunk/engine/src/test/java/com/metamatrix/query/rewriter/TestQueryRewriter.java trunk/metadata/src/main/java/org/teiid/connector/metadata/MetadataResultsPostProcessor.java trunk/metadata/src/main/java/org/teiid/metadata/index/CharOperation.java trunk/metadata/src/test/java/com/metamatrix/connector/metadata/index/TestMetadataResultsPostProcessor.java trunk/test-integration/src/test/java/org/teiid/connector/language/TestLanguageUtil.java Log: TEIID-748 updating the queryrewriter handling of possible null values. Modified: trunk/connectors/connector-ldap/src/main/java/com/metamatrix/connector/ldap/IQueryToLdapSearchParser.java =================================================================== --- trunk/connectors/connector-ldap/src/main/java/com/metamatrix/connector/ldap/IQueryToLdapSearchParser.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/connectors/connector-ldap/src/main/java/com/metamatrix/connector/ldap/IQueryToLdapSearchParser.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -476,7 +476,7 @@ while(itr.hasNext()) { ICriteria c = (ICriteria)itr.next(); // recurse on each criterion - filterList = getSearchFilterFromWhereClause(c, filterList); + filterList.addAll(getSearchFilterFromWhereClause(c, new LinkedList())); } filterList.add(")"); //$NON-NLS-1$ @@ -527,7 +527,7 @@ } else if (criteria instanceof INotCriteria) { logger.logTrace("Parsing NOT criteria."); //$NON-NLS-1$ isNegated = true; - filterList = getSearchFilterFromWhereClause(((INotCriteria)criteria).getCriteria(), filterList); + filterList.addAll(getSearchFilterFromWhereClause(((INotCriteria)criteria).getCriteria(), new LinkedList())); } if (isNegated) { Modified: trunk/connectors/connector-ldap/src/test/java/com/metamatrix/connector/ldap/TestIQueryToLdapSearchParser.java =================================================================== --- trunk/connectors/connector-ldap/src/test/java/com/metamatrix/connector/ldap/TestIQueryToLdapSearchParser.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/connectors/connector-ldap/src/test/java/com/metamatrix/connector/ldap/TestIQueryToLdapSearchParser.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -191,7 +191,7 @@ // Set Expected SearchDetails Values //----------------------------------- String expectedContextName = "ou=people,dc=metamatrix,dc=com"; //$NON-NLS-1$ - String expectedContextFilter = "(!(|(cn=R*)(cn=S*)))"; //$NON-NLS-1$ + String expectedContextFilter = "(&(!(cn=R*))(!(cn=S*)))"; //$NON-NLS-1$ List expectedAttrNameList = new ArrayList(); expectedAttrNameList.add("uid"); //$NON-NLS-1$ Modified: trunk/connectors/connector-salesforce/src/test/java/com/metamatrix/connector/salesforce/execution/visitors/TestVisitors.java =================================================================== --- trunk/connectors/connector-salesforce/src/test/java/com/metamatrix/connector/salesforce/execution/visitors/TestVisitors.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/connectors/connector-salesforce/src/test/java/com/metamatrix/connector/salesforce/execution/visitors/TestVisitors.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -89,7 +89,7 @@ IQuery command = (IQuery)translationUtility.parseCommand("select * from Account where not (Name = 'foo' and Stuff = 'bar')"); //$NON-NLS-1$ SelectVisitor visitor = new SelectVisitor(translationUtility.createRuntimeMetadata()); visitor.visit(command); - assertEquals("SELECT id, AccountName, Stuff FROM Account WHERE NOT ((AccountName = 'foo') AND (Stuff = 'bar'))", visitor.getQuery().toString().trim()); //$NON-NLS-1$ + assertEquals("SELECT id, AccountName, Stuff FROM Account WHERE (AccountName != 'foo') OR (Stuff != 'bar')", visitor.getQuery().toString().trim()); //$NON-NLS-1$ } @Test public void testCountStart() throws Exception { Modified: trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -116,6 +116,7 @@ import com.metamatrix.query.sql.lang.UnaryFromClause; import com.metamatrix.query.sql.lang.Update; import com.metamatrix.query.sql.lang.XQuery; +import com.metamatrix.query.sql.lang.PredicateCriteria.Negatable; import com.metamatrix.query.sql.navigator.PostOrderNavigator; import com.metamatrix.query.sql.navigator.PreOrderNavigator; import com.metamatrix.query.sql.proc.AssignmentStatement; @@ -186,7 +187,7 @@ private ProcessorDataManager dataMgr; private Map variables; //constant propagation private int commandType; - + private QueryRewriter(QueryMetadataInterface metadata, CommandContext context, CreateUpdateProcedureCommand procCommand) { this.metadata = metadata; @@ -314,23 +315,17 @@ return result; } - private Option mergeOptions( Option sourceOption, Option targetOption ) { - if ( sourceOption == null ) { - return targetOption; - } - if ( sourceOption.getPlanOnly() == true ) { + private void mergeOptions( Option sourceOption, Option targetOption ) { + if ( sourceOption.getPlanOnly()) { targetOption.setPlanOnly( true ); } - if ( sourceOption.getDebug() == true ) { + if ( sourceOption.getDebug()) { targetOption.setDebug( true ); } - if ( sourceOption.getShowPlan() == true ) { + if ( sourceOption.getShowPlan()) { targetOption.setShowPlan( true ); } - - return targetOption; } - private Command removeProceduralWrapper(Command command) throws QueryValidatorException { @@ -361,7 +356,7 @@ return command; } try { - Object value = Evaluator.evaluate(expr); + Object value = new Evaluator(Collections.emptyMap(), this.dataMgr, context).evaluate(expr, Collections.emptyList()); //check contraint if (value == null && !metadata.elementSupports(param.getMetadataID(), SupportConstants.Element.NULL)) { @@ -388,11 +383,9 @@ if (child != null && child.getType() != Command.TYPE_DYNAMIC) { if ( child.getOption() == null ) { child.setOption( command.getOption() ); - } else { - Option merged = mergeOptions( command.getOption(), child.getOption() ); - child.setOption(merged); + } else if (command.getOption() != null) { + mergeOptions( command.getOption(), child.getOption() ); } - return child; } return command; @@ -438,8 +431,7 @@ case Statement.TYPE_IF: IfStatement ifStmt = (IfStatement) statement; Criteria ifCrit = ifStmt.getCondition(); - Criteria evalCrit = rewriteCriteria(ifCrit, false); - evalCrit = evaluateCriteria(evalCrit); + Criteria evalCrit = rewriteCriteria(ifCrit); ifStmt.setCondition(evalCrit); if(evalCrit.equals(TRUE_CRITERIA)) { @@ -507,13 +499,12 @@ case Statement.TYPE_WHILE: WhileStatement whileStatement = (WhileStatement) statement; Criteria crit = whileStatement.getCondition(); - crit = rewriteCriteria(crit, false); - evalCrit = evaluateCriteria(crit); + crit = rewriteCriteria(crit); - whileStatement.setCondition(evalCrit); - if(evalCrit.equals(TRUE_CRITERIA)) { + whileStatement.setCondition(crit); + if(crit.equals(TRUE_CRITERIA)) { throw new QueryValidatorException(QueryExecPlugin.Util.getString("QueryRewriter.infinite_while")); //$NON-NLS-1$ - } else if(evalCrit.equals(FALSE_CRITERIA) || evalCrit.equals(UNKNOWN_CRITERIA)) { + } else if(crit.equals(FALSE_CRITERIA) || crit.equals(UNKNOWN_CRITERIA)) { return null; } whileStatement.setBlock(rewriteBlock(whileStatement.getBlock())); @@ -571,7 +562,7 @@ hasCritElmts = selector.getElements(); // collect elements present on the user's criteria and check if // all of the hasCriteria elements are among them - Collection userElmnts = ElementCollectorVisitor.getElements(userCrit, true); + Collection<ElementSymbol> userElmnts = ElementCollectorVisitor.getElements(userCrit, true); if(!userElmnts.containsAll(hasCritElmts)) { return FALSE_CRITERIA; } @@ -590,7 +581,7 @@ Criteria predicateCriteria = (Criteria) criteriaIter.next(); // atleast one of the hasElemnets should be on this predicate else // proceed to the next predicate - Collection predElmnts = ElementCollectorVisitor.getElements(predicateCriteria, true); + Collection<ElementSymbol> predElmnts = ElementCollectorVisitor.getElements(predicateCriteria, true); if(selector.hasElements()) { Iterator hasIter = hasCritElmts.iterator(); boolean containsElmnt = false; @@ -701,7 +692,7 @@ translatedCriteria = translateVisitor.getTranslatedCriteria(); ((TranslatableProcedureContainer)userCmd).addImplicitParameters(translateVisitor.getImplicitParams()); - translatedCriteria = rewriteCriteria(translatedCriteria, false); + translatedCriteria = rewriteCriteria(translatedCriteria); // apply any implicit conversions try { @@ -732,7 +723,7 @@ // Rewrite criteria Criteria crit = query.getCriteria(); if(crit != null) { - crit = rewriteCriteria(crit, false); + crit = rewriteCriteria(crit); if(crit == TRUE_CRITERIA) { query.setCriteria(null); } else { @@ -817,7 +808,7 @@ // Rewrite having Criteria having = query.getHaving(); if(having != null) { - query.setHaving(rewriteCriteria(having, false)); + query.setHaving(rewriteCriteria(having)); } rewriteExpressions(query.getSelect()); @@ -1018,7 +1009,7 @@ //rewrite join crits by rewriting a compound criteria Criteria criteria = new CompoundCriteria(new ArrayList(joinCrits)); joinCrits.clear(); - criteria = rewriteCriteria(criteria, false); + criteria = rewriteCriteria(criteria); if (criteria instanceof CompoundCriteria && ((CompoundCriteria)criteria).getOperator() == CompoundCriteria.AND) { joinCrits.addAll(((CompoundCriteria)criteria).getCriteria()); } else { @@ -1052,7 +1043,7 @@ * @return The re-written criteria */ public static Criteria rewriteCriteria(Criteria criteria, CreateUpdateProcedureCommand procCommand, CommandContext context, QueryMetadataInterface metadata) throws QueryValidatorException { - return new QueryRewriter(metadata, context, procCommand).rewriteCriteria(criteria, false); + return new QueryRewriter(metadata, context, procCommand).rewriteCriteria(criteria); } /** @@ -1062,9 +1053,9 @@ * in the procedural language. * @return The re-written criteria */ - private Criteria rewriteCriteria(Criteria criteria, boolean preserveUnknown) throws QueryValidatorException { - if(criteria instanceof CompoundCriteria) { - return rewriteCriteria((CompoundCriteria)criteria, true, preserveUnknown); + private Criteria rewriteCriteria(Criteria criteria) throws QueryValidatorException { + if(criteria instanceof CompoundCriteria) { + return rewriteCriteria((CompoundCriteria)criteria, true); } else if(criteria instanceof NotCriteria) { criteria = rewriteCriteria((NotCriteria)criteria); } else if(criteria instanceof CompareCriteria) { @@ -1078,7 +1069,7 @@ } else if(criteria instanceof IsNullCriteria) { criteria = rewriteCriteria((IsNullCriteria)criteria); } else if(criteria instanceof BetweenCriteria) { - criteria = rewriteCriteria((BetweenCriteria)criteria, preserveUnknown); + criteria = rewriteCriteria((BetweenCriteria)criteria); } else if(criteria instanceof HasCriteria) { criteria = rewriteCriteria((HasCriteria)criteria); } else if(criteria instanceof TranslateCriteria) { @@ -1094,7 +1085,7 @@ } else if (criteria instanceof DependentSetCriteria) { criteria = rewriteCriteria((AbstractSetCriteria)criteria); } - + return evaluateCriteria(criteria); } @@ -1106,7 +1097,7 @@ */ public static Criteria optimizeCriteria(CompoundCriteria criteria) { try { - return new QueryRewriter(null, null, null).rewriteCriteria(criteria, false, false); + return new QueryRewriter(null, null, null).rewriteCriteria(criteria, false); } catch (QueryValidatorException err) { //shouldn't happen return criteria; @@ -1116,20 +1107,17 @@ /** May be simplified if this is an AND and a sub criteria is always * false or if this is an OR and a sub criteria is always true */ - private Criteria rewriteCriteria(CompoundCriteria criteria, boolean rewrite, boolean preserveUnknown) throws QueryValidatorException { - List crits = criteria.getCriteria(); + private Criteria rewriteCriteria(CompoundCriteria criteria, boolean rewrite) throws QueryValidatorException { + List<Criteria> crits = criteria.getCriteria(); int operator = criteria.getOperator(); // Walk through crits and collect converted ones - LinkedHashSet newCrits = new LinkedHashSet(crits.size()); - Iterator critIter = crits.iterator(); - while(critIter.hasNext()) { - Criteria converted = (Criteria) critIter.next(); + LinkedHashSet<Criteria> newCrits = new LinkedHashSet<Criteria>(crits.size()); + for (Criteria converted : crits) { if (rewrite) { - converted = rewriteCriteria(converted, preserveUnknown); - converted = evaluateCriteria(converted); + converted = rewriteCriteria(converted); } else if (converted instanceof CompoundCriteria) { - converted = rewriteCriteria((CompoundCriteria)converted, false, preserveUnknown); + converted = rewriteCriteria((CompoundCriteria)converted, false); } //begin boolean optimizations @@ -1143,25 +1131,20 @@ // this AND must be false as at least one branch is always false return converted; } + } else if (converted == UNKNOWN_CRITERIA) { + if (operator == CompoundCriteria.AND) { + return FALSE_CRITERIA; + } + continue; } else { if (converted instanceof CompoundCriteria) { CompoundCriteria other = (CompoundCriteria)converted; if (other.getOperator() == criteria.getOperator()) { - Iterator i = other.getCriteria().iterator(); - while (i.hasNext()) { - newCrits.add(i.next()); - } + newCrits.addAll(other.getCriteria()); continue; } } - //if we're not interested in preserving unknowns, then treat unknown as false - if (!preserveUnknown && converted == UNKNOWN_CRITERIA) { - if (operator == CompoundCriteria.AND) { - return FALSE_CRITERIA; - } - } else { - newCrits.add(converted); - } + newCrits.add(converted); } } @@ -1172,7 +1155,7 @@ return FALSE_CRITERIA; } else if(newCrits.size() == 1) { // Only one sub crit now, so just return it - return (Criteria) newCrits.iterator().next(); + return newCrits.iterator().next(); } else { criteria.getCriteria().clear(); criteria.getCriteria().addAll(newCrits); @@ -1183,7 +1166,7 @@ private Criteria evaluateCriteria(Criteria crit) throws QueryValidatorException { if(EvaluatableVisitor.isFullyEvaluatable(crit, true)) { try { - Boolean eval = new Evaluator(Collections.emptyMap(), null, null).evaluateTVL(crit, Collections.emptyList()); + Boolean eval = new Evaluator(Collections.emptyMap(), this.dataMgr, context).evaluateTVL(crit, Collections.emptyList()); if (eval == null) { return UNKNOWN_CRITERIA; @@ -1204,12 +1187,21 @@ return crit; } - private Criteria rewriteCriteria(NotCriteria criteria) throws QueryValidatorException { - Criteria innerCrit = rewriteCriteria(criteria.getCriteria(), true); - - innerCrit = evaluateCriteria(innerCrit); + Criteria innerCrit = criteria.getCriteria(); + if (innerCrit instanceof CompoundCriteria) { + //reduce to only negation of predicates, so that the null/unknown handling criteria is applied appropriately + return rewriteCriteria(Criteria.toConjunctiveNormalForm(criteria)); + } + if (innerCrit instanceof Negatable) { + ((Negatable) innerCrit).negate(); + return rewriteCriteria(innerCrit); + } + if (innerCrit instanceof NotCriteria) { + return rewriteCriteria(((NotCriteria)innerCrit).getCriteria()); + } + innerCrit = rewriteCriteria(innerCrit); if(innerCrit == TRUE_CRITERIA) { return FALSE_CRITERIA; } else if(innerCrit == FALSE_CRITERIA) { @@ -1217,11 +1209,6 @@ } else if (innerCrit == UNKNOWN_CRITERIA) { return UNKNOWN_CRITERIA; } - - if (innerCrit instanceof NotCriteria) { - return ((NotCriteria)innerCrit).getCriteria(); - } - criteria.setCriteria(innerCrit); return criteria; } @@ -1232,7 +1219,7 @@ * @return * @throws QueryValidatorException */ - private Criteria rewriteCriteria(BetweenCriteria criteria, boolean preserveUnknown) throws QueryValidatorException { + private Criteria rewriteCriteria(BetweenCriteria criteria) throws QueryValidatorException { CompareCriteria lowerCriteria = new CompareCriteria(criteria.getExpression(), criteria.isNegated() ? CompareCriteria.LT: CompareCriteria.GE, criteria.getLowerExpression()); @@ -1243,14 +1230,23 @@ lowerCriteria, upperCriteria); - return rewriteCriteria(newCriteria, preserveUnknown); + return rewriteCriteria(newCriteria); } private Criteria rewriteCriteria(CompareCriteria criteria) throws QueryValidatorException { Expression leftExpr = rewriteExpressionDirect(criteria.getLeftExpression()); Expression rightExpr = rewriteExpressionDirect(criteria.getRightExpression()); + criteria.setLeftExpression(leftExpr); + criteria.setRightExpression(rightExpr); - if(!EvaluatableVisitor.willBecomeConstant(rightExpr) && EvaluatableVisitor.willBecomeConstant(leftExpr)) { + if (isNull(leftExpr) || isNull(rightExpr)) { + return UNKNOWN_CRITERIA; + } + + boolean rightConstant = false; + if(EvaluatableVisitor.willBecomeConstant(rightExpr)) { + rightConstant = true; + } else if (EvaluatableVisitor.willBecomeConstant(leftExpr)) { // Swap in this particular case for connectors criteria.setLeftExpression(rightExpr); criteria.setRightExpression(leftExpr); @@ -1262,19 +1258,18 @@ case CompareCriteria.GT: criteria.setOperator(CompareCriteria.LT); break; case CompareCriteria.GE: criteria.setOperator(CompareCriteria.LE); break; } - - } else { - criteria.setLeftExpression(leftExpr); - criteria.setRightExpression(rightExpr); - } - - if(criteria.getLeftExpression() instanceof Function && EvaluatableVisitor.willBecomeConstant(criteria.getRightExpression())) { - criteria = simplifyWithInverse(criteria); - } + rightConstant = true; + } - if (isNull(criteria.getLeftExpression()) || isNull(criteria.getRightExpression())) { - return UNKNOWN_CRITERIA; - } + Function f = null; + while (rightConstant && f != criteria.getLeftExpression() && criteria.getLeftExpression() instanceof Function) { + f = (Function)criteria.getLeftExpression(); + Criteria result = simplifyWithInverse(criteria); + if (!(result instanceof CompareCriteria)) { + return result; + } + criteria = (CompareCriteria)result; + } Criteria modCriteria = simplifyTimestampMerge(criteria); if(modCriteria instanceof CompareCriteria) { @@ -1310,7 +1305,7 @@ return criteria; } - private CompareCriteria simplifyWithInverse(CompareCriteria criteria) throws QueryValidatorException { + private Criteria simplifyWithInverse(CompareCriteria criteria) throws QueryValidatorException { Expression leftExpr = criteria.getLeftExpression(); Function leftFunction = (Function) leftExpr; @@ -1465,10 +1460,6 @@ criteria.setRightExpression(combinedConst); criteria.setOperator(operator); - if (expr instanceof Function) { - return simplifyWithInverse(criteria); - } - // Return new simplified criteria return criteria; } @@ -1496,17 +1487,13 @@ * @throws QueryValidatorException * @since 4.2 */ - private CompareCriteria simplifyConvertFunction(CompareCriteria crit) throws QueryValidatorException { + private Criteria simplifyConvertFunction(CompareCriteria crit) throws QueryValidatorException { Function leftFunction = (Function) crit.getLeftExpression(); Constant rightConstant = (Constant) crit.getRightExpression(); Expression leftExpr = leftFunction.getArgs()[0]; String leftExprTypeName = DataTypeManager.getDataTypeName(leftExpr.getType()); - if (leftExpr.getType() == DataTypeManager.DefaultDataClasses.NULL) { - return crit; - } - Constant result = null; try { result = ResolverUtil.convertConstant(DataTypeManager.getDataTypeName(rightConstant.getType()), leftExprTypeName, rightConstant); @@ -1515,19 +1502,12 @@ } if (result == null) { - if (crit.getOperator() == CompareCriteria.EQ) { - return FALSE_CRITERIA; - } - return TRUE_CRITERIA; + return getSimpliedCriteria(crit, leftExpr, crit.getOperator() != CompareCriteria.EQ, true); } - crit.setRightExpression(result); + crit.setRightExpression(result); crit.setLeftExpression(leftExpr); - if (leftExpr instanceof Function) { - return simplifyWithInverse(crit); - } - return crit; } @@ -1595,14 +1575,17 @@ if (!removedSome) { return crit; //just return as is } - return rewriteCriteria(crit); + } else { + if (newValues.isEmpty()) { + return getSimpliedCriteria(crit, leftExpr, !crit.isNegated(), true); + } + crit.setExpression(leftExpr); + crit.setValues(newValues); } - crit.setExpression(leftExpr); - crit.setValues(newValues); return rewriteCriteria(crit); } - private CompareCriteria simplifyParseFormatFunction(CompareCriteria crit) throws QueryValidatorException { + private Criteria simplifyParseFormatFunction(CompareCriteria crit) throws QueryValidatorException { Function leftFunction = (Function) crit.getLeftExpression(); String funcName = leftFunction.getName().toLowerCase(); String inverseFunction = null; @@ -1668,10 +1651,7 @@ result = funcLib.invokeFunction(reverseFunction, new Object[] { result, format } ); if (!example.equals(result)) { if (parseFunction) { - if (crit.getOperator() == CompareCriteria.EQ) { - return FALSE_CRITERIA; - } - return TRUE_CRITERIA; + return getSimpliedCriteria(crit, leftExpr, crit.getOperator() != CompareCriteria.EQ, true); } //the format is loosing information, so it must not be invertable return crit; @@ -1706,9 +1686,6 @@ //Not all numeric formats are invertable, so just return the criteria as it may still be valid return crit; } - if (leftExpr instanceof Function) { - return simplifyWithInverse(crit); - } return crit; } @@ -1880,40 +1857,49 @@ } Expression rightExpr = criteria.getRightExpression(); - if(rightExpr instanceof Constant) { + if(rightExpr instanceof Constant && rightExpr.getType().equals(DataTypeManager.DefaultDataClasses.STRING)) { Constant constant = (Constant) rightExpr; - if(constant.getType().equals(DataTypeManager.DefaultDataClasses.STRING)) { - String value = (String) constant.getValue(); + String value = (String) constant.getValue(); - char escape = criteria.getEscapeChar(); + char escape = criteria.getEscapeChar(); - // Check whether escape char is unnecessary and remove it - if(escape != MatchCriteria.NULL_ESCAPE_CHAR) { - if(value.indexOf(escape) < 0) { - criteria.setEscapeChar(MatchCriteria.NULL_ESCAPE_CHAR); - } - } + // Check whether escape char is unnecessary and remove it + if(escape != MatchCriteria.NULL_ESCAPE_CHAR && value.indexOf(escape) < 0) { + criteria.setEscapeChar(MatchCriteria.NULL_ESCAPE_CHAR); + } - // if the value of this string constant is '%', then we know the crit will - // always be true - if ( value.equals( String.valueOf(MatchCriteria.WILDCARD_CHAR)) ) { - return criteria.isNegated()?FALSE_CRITERIA:TRUE_CRITERIA; - } - - // if both left and right expressions are strings, and the LIKE match characters ('*', '_') are not present - // in the right expression, rewrite the criteria as EQUALs rather than LIKE - if(DataTypeManager.DefaultDataClasses.STRING.equals(criteria.getLeftExpression().getType()) && value.indexOf(escape) < 0 && value.indexOf(MatchCriteria.MATCH_CHAR) < 0 && value.indexOf(MatchCriteria.WILDCARD_CHAR) < 0) { - if (value.equals(MatchCriteria.WILDCARD_CHAR)) { - return TRUE_CRITERIA; - } - return rewriteCriteria(new CompareCriteria(criteria.getLeftExpression(), criteria.isNegated()?CompareCriteria.NE:CompareCriteria.EQ, criteria.getRightExpression())); - } + // if the value of this string constant is '%', then we know the crit will + // always be true + if ( value.equals( String.valueOf(MatchCriteria.WILDCARD_CHAR)) ) { + return getSimpliedCriteria(criteria, criteria.getLeftExpression(), !criteria.isNegated(), true); + } + + // if both left and right expressions are strings, and the LIKE match characters ('*', '_') are not present + // in the right expression, rewrite the criteria as EQUALs rather than LIKE + if(DataTypeManager.DefaultDataClasses.STRING.equals(criteria.getLeftExpression().getType()) && value.indexOf(escape) < 0 && value.indexOf(MatchCriteria.MATCH_CHAR) < 0 && value.indexOf(MatchCriteria.WILDCARD_CHAR) < 0) { + return rewriteCriteria(new CompareCriteria(criteria.getLeftExpression(), criteria.isNegated()?CompareCriteria.NE:CompareCriteria.EQ, criteria.getRightExpression())); } } return criteria; } + private Criteria getSimpliedCriteria(Criteria crit, Expression a, boolean outcome, boolean nullPossible) { + if (nullPossible) { + if (outcome) { + if (this.dataMgr != null) { + return crit; + } + IsNullCriteria inc = new IsNullCriteria(a); + inc.setNegated(true); + return inc; + } + } else if (outcome) { + return TRUE_CRITERIA; + } + return FALSE_CRITERIA; + } + private Criteria rewriteCriteria(AbstractSetCriteria criteria) throws QueryValidatorException { criteria.setExpression(rewriteExpressionDirect(criteria.getExpression())); @@ -1924,7 +1910,6 @@ return criteria; } - private Criteria rewriteCriteria(SetCriteria criteria) throws QueryValidatorException { criteria.setExpression(rewriteExpressionDirect(criteria.getExpression())); @@ -2202,8 +2187,8 @@ if(exprs.length == 3){ decodeDelimiter = (String)((Constant)exprs[2]).getValue(); } - List newWhens = new ArrayList(); - List newThens = new ArrayList(); + List<Criteria> newWhens = new ArrayList<Criteria>(); + List<Constant> newThens = new ArrayList<Constant>(); Constant elseConst = null; StringTokenizer tokenizer = new StringTokenizer(decodeString, decodeDelimiter); while (tokenizer.hasMoreTokens()) { @@ -2295,13 +2280,13 @@ private Expression rewriteCaseExpression(SearchedCaseExpression expr) throws QueryValidatorException { int whenCount = expr.getWhenCount(); - ArrayList whens = new ArrayList(whenCount); - ArrayList thens = new ArrayList(whenCount); + ArrayList<Criteria> whens = new ArrayList<Criteria>(whenCount); + ArrayList<Expression> thens = new ArrayList<Expression>(whenCount); for (int i = 0; i < whenCount; i++) { // Check the when to see if this CASE can be rewritten due to an always true/false when - Criteria rewrittenWhen = rewriteCriteria(expr.getWhenCriteria(i), false); + Criteria rewrittenWhen = rewriteCriteria(expr.getWhenCriteria(i)); if(rewrittenWhen == TRUE_CRITERIA) { // WHEN is always true, so just return the THEN return rewriteExpressionDirect(expr.getThenExpression(i)); @@ -2521,7 +2506,7 @@ // Rewrite criteria Criteria crit = update.getCriteria(); if(crit != null) { - update.setCriteria(rewriteCriteria(crit, false)); + update.setCriteria(rewriteCriteria(crit)); } return update; @@ -2563,7 +2548,7 @@ // Rewrite criteria Criteria crit = delete.getCriteria(); if(crit != null) { - delete.setCriteria(rewriteCriteria(crit, false)); + delete.setCriteria(rewriteCriteria(crit)); } return delete; Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractCompareCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractCompareCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractCompareCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -65,27 +65,6 @@ */ private int operator = EQ; -// /** -// * Constructs a default instance of this class. -// */ -// public AbstractCompareCriteria() { -// super(); -// } - -// /** -// * Constructs an instance of this class for a specific "operand operator -// * operand" clause. -// * -// * @param identifier The variable being compared -// * @param value The value the variable is being compared to (literal or variable) -// * @param operator The operator representing how the variable and value are to -// * be compared -// */ -// public AbstractCompareCriteria( Expression leftExpression, int operator, Expression rightExpression ) { -// super(); -// set(leftExpression, operator, rightExpression); -// } - /** * Returns the operator. * @return The operator @@ -154,6 +133,5 @@ default: return "??"; //$NON-NLS-1$ } } - - + } // END CLASS Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractSetCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractSetCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/AbstractSetCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -22,6 +22,7 @@ package com.metamatrix.query.sql.lang; +import com.metamatrix.query.sql.lang.PredicateCriteria.Negatable; import com.metamatrix.query.sql.symbol.Expression; /** @@ -29,7 +30,7 @@ * IN criteria: {@link SetCriteria} (where values are specified) and {@link SubquerySetCriteria} * (where a subquery is defined and will supply the values for the IN set). */ -public abstract class AbstractSetCriteria extends PredicateCriteria { +public abstract class AbstractSetCriteria extends PredicateCriteria implements Negatable { /** The left expression */ private Expression expression; @@ -75,6 +76,11 @@ public void setNegated(boolean negationFlag) { negated = negationFlag; } + + @Override + public void negate() { + this.negated = !this.negated; + } /** * Deep copy of object Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/CompareCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/CompareCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/CompareCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -25,6 +25,7 @@ import com.metamatrix.core.util.EquivalenceUtil; import com.metamatrix.core.util.HashCodeUtil; import com.metamatrix.query.sql.LanguageVisitor; +import com.metamatrix.query.sql.lang.PredicateCriteria.Negatable; import com.metamatrix.query.sql.symbol.Expression; /** @@ -40,7 +41,7 @@ * <LI>5 &lt;= length(companyName)</LI> * </UL> */ -public class CompareCriteria extends AbstractCompareCriteria { +public class CompareCriteria extends AbstractCompareCriteria implements Negatable { /** The right-hand expression. */ private Expression rightExpression; @@ -172,4 +173,21 @@ return result; } + @Override + public void negate() { + this.setOperator(getInverseOperator(this.getOperator())); + } + + public static int getInverseOperator(int op) { + switch ( op ) { + case EQ: return NE; + case NE: return EQ; + case LT: return GE; + case GT: return LE; + case LE: return GT; + case GE: return LT; + default: return -1; + } + } + } // END CLASS Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/IsNullCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/IsNullCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/IsNullCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -23,17 +23,18 @@ package com.metamatrix.query.sql.lang; import com.metamatrix.query.sql.*; +import com.metamatrix.query.sql.lang.PredicateCriteria.Negatable; import com.metamatrix.query.sql.symbol.Expression; import com.metamatrix.core.util.EquivalenceUtil; /** * Represents criteria such as: "<expression> IS NULL". */ -public class IsNullCriteria extends PredicateCriteria { +public class IsNullCriteria extends PredicateCriteria implements Negatable { private Expression expression; /** Negation flag. Indicates whether the criteria expression contains a NOT. */ - private boolean negated = false; + private boolean negated; /** * Constructs a default instance of this class. @@ -79,6 +80,11 @@ public void setNegated(boolean negationFlag) { negated = negationFlag; } + + @Override + public void negate() { + this.negated = !this.negated; + } public void acceptVisitor(LanguageVisitor visitor) { visitor.visit(this); Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/MatchCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/MatchCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/MatchCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -29,6 +29,7 @@ import com.metamatrix.core.util.HashCodeUtil; import com.metamatrix.query.QueryPlugin; import com.metamatrix.query.sql.LanguageVisitor; +import com.metamatrix.query.sql.lang.PredicateCriteria.Negatable; import com.metamatrix.query.sql.symbol.Expression; /** @@ -38,7 +39,7 @@ * match character. The escape character can be used to escape an actual % or _ within a * match string. */ -public class MatchCriteria extends PredicateCriteria { +public class MatchCriteria extends PredicateCriteria implements Negatable { /** The default wildcard character - '%' */ public static final char WILDCARD_CHAR = '%'; @@ -158,6 +159,11 @@ public void setNegated(boolean negationFlag) { negated = negationFlag; } + + @Override + public void negate() { + this.negated = !this.negated; + } public void acceptVisitor(LanguageVisitor visitor) { visitor.visit(this); Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/lang/PredicateCriteria.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/lang/PredicateCriteria.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/lang/PredicateCriteria.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -34,6 +34,12 @@ */ public abstract class PredicateCriteria extends Criteria { + public interface Negatable { + + void negate(); + + } + /** * Constructs a default instance of this class. */ Modified: trunk/engine/src/test/java/com/metamatrix/query/rewriter/TestQueryRewriter.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/rewriter/TestQueryRewriter.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/engine/src/test/java/com/metamatrix/query/rewriter/TestQueryRewriter.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -203,11 +203,11 @@ } @Test public void testRewriteUnknown6() { - helpTestRewriteCriteria("not(pm1.g1.e1 = '1' and '1' = convert(null, string))", "NOT ((pm1.g1.e1 = '1') and (NULL <> NULL))"); //$NON-NLS-1$ //$NON-NLS-2$ + helpTestRewriteCriteria("not(pm1.g1.e1 = '1' and '1' = convert(null, string))", "pm1.g1.e1 <> '1'"); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testRewriteUnknown7() { - helpTestRewriteCriteria("not(pm1.g1.e1 = '1' or '1' = convert(null, string))", "NOT ((pm1.g1.e1 = '1') or (NULL <> NULL))"); //$NON-NLS-1$ //$NON-NLS-2$ + helpTestRewriteCriteria("not(pm1.g1.e1 = '1' or '1' = convert(null, string))", "1 = 0"); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testRewriteUnknown8() { @@ -224,7 +224,11 @@ @Test public void testRewriteInCriteriaWithSingleValue1() { helpTestRewriteCriteria("pm1.g1.e1 not in ('1')", "pm1.g1.e1 != '1'"); //$NON-NLS-1$ //$NON-NLS-2$ - } + } + + @Test public void testRewriteInCriteriaWithConvert() { + helpTestRewriteCriteria("convert(pm1.g1.e2, string) not in ('x')", "pm1.g1.e2 IS NOT NULL"); //$NON-NLS-1$ //$NON-NLS-2$ + } @Test public void testRewriteInCriteriaWithNoValues() throws Exception { Criteria crit = new SetCriteria(new ElementSymbol("e1"), Collections.EMPTY_LIST); //$NON-NLS-1$ @@ -435,7 +439,7 @@ @Test public void testRewriteCrit_parseTimestamp3() { helpTestRewriteCriteria("PARSETimestamp(pm3.g1.e1, 'yyyy dd mm') <> {ts'2003-05-01 13:25:04.5'}", //$NON-NLS-1$ - "1 = 1" ); //$NON-NLS-1$ + "pm3.g1.e1 is not null" ); //$NON-NLS-1$ } @Test public void testRewriteCrit_parseTimestamp4() { @@ -743,7 +747,7 @@ } @Test public void testRewriteNot3() { - helpTestRewriteCriteria("NOT (pm1.g1.e1='x')", "NOT (pm1.g1.e1 = 'x')"); //$NON-NLS-1$ //$NON-NLS-2$ + helpTestRewriteCriteria("NOT (pm1.g1.e1='x')", "pm1.g1.e1 <> 'x'"); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testRewriteDefect1() { @@ -1597,7 +1601,7 @@ /** Check that this returns true, x is not convertable to an int */ @Test public void testRewriteCase1954f1() { - helpTestRewriteCriteria("convert(pm1.g1.e2, string) != 'x'", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$ + helpTestRewriteCriteria("convert(pm1.g1.e2, string) != 'x'", "pm1.g1.e2 is not null"); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testRewriteCase1954Set() { @@ -1957,16 +1961,16 @@ new Class[] { DataTypeManager.DefaultDataClasses.STRING}); } - private void verifyProjectedTypesOnUnionBranches(String unionQuery, Class[] types) throws QueryValidatorException, QueryParserException, QueryResolverException, MetaMatrixComponentException { + private void verifyProjectedTypesOnUnionBranches(String unionQuery, Class<?>[] types) throws QueryValidatorException, QueryParserException, QueryResolverException, MetaMatrixComponentException { SetQuery union = (SetQuery)QueryParser.getQueryParser().parseCommand(unionQuery); QueryResolver.resolveCommand(union, FakeMetadataFactory.example1Cached()); union = (SetQuery)QueryRewriter.rewrite(union, null, FakeMetadataFactory.example1Cached(), null); for (QueryCommand query : union.getQueryCommands()) { - List projSymbols = query.getProjectedSymbols(); + List<SingleElementSymbol> projSymbols = query.getProjectedSymbols(); for(int i=0; i<projSymbols.size(); i++) { - assertEquals("Found type mismatch at column " + i, types[i], ((SingleElementSymbol) projSymbols.get(i)).getType()); //$NON-NLS-1$ + assertEquals("Found type mismatch at column " + i, types[i], projSymbols.get(i).getType()); //$NON-NLS-1$ } } } @@ -2074,16 +2078,14 @@ helpTestRewriteCriteria("coalesce(convert(pm1.g1.e2, double), pm1.g1.e4) = 1", "ifnull(convert(pm1.g1.e2, double), pm1.g1.e4) = 1", true); //$NON-NLS-1$ //$NON-NLS-2$ } - @Test public void testProcWithNull() throws Exception { + /** + * Should fail since null is not allowed as an input + * @throws Exception + */ + @Test(expected=QueryValidatorException.class) public void testProcWithNull() throws Exception { String sql = "exec pm1.vsp26(1, null)"; //$NON-NLS-1$ - try { - helpTestRewriteCommand(sql, "", FakeMetadataFactory.example1Cached()); - fail("expected exception"); - } catch (QueryValidatorException e) { - - } - + helpTestRewriteCommand(sql, "", FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ } /** @@ -2201,11 +2203,46 @@ assertEquals( "e2 <= 5", ccrit.getCriteria(1).toString() ); //$NON-NLS-1$ } - @Test public void testRewriteLike() { + @Test public void testRewriteNullHandling() { String original = "pm1.g1.e1 like '%'"; //$NON-NLS-1$ - String expected = "1 = 1"; //$NON-NLS-1$ + String expected = "pm1.g1.e1 is not null"; //$NON-NLS-1$ helpTestRewriteCriteria(original, expected); } + @Test public void testRewriteNullHandling1() { + String original = "not(pm1.g1.e1 like '%' or pm1.g1.e1 = '1')"; //$NON-NLS-1$ + String expected = "1 = 0"; //$NON-NLS-1$ + + helpTestRewriteCriteria(original, expected); + } + + @Test public void testRewriteNullHandling2() { + String original = "not(pm1.g1.e1 like '%' and pm1.g1.e1 = '1')"; //$NON-NLS-1$ + String expected = "pm1.g1.e1 <> '1'"; //$NON-NLS-1$ + + helpTestRewriteCriteria(original, expected); + } + + @Test public void testRewriteNullHandling3() { + String original = "pm1.g1.e1 like '%' or pm1.g1.e1 = '1'"; //$NON-NLS-1$ + String expected = "(pm1.g1.e1 IS NOT NULL) OR (pm1.g1.e1 = '1')"; //$NON-NLS-1$ + + helpTestRewriteCriteria(original, expected); + } + + @Test public void testRewriteNullHandling4() { + String original = "not((pm1.g1.e1 like '%' or pm1.g1.e1 = '1') and pm1.g1.e2 < 5)"; //$NON-NLS-1$ + String expected = "(pm1.g1.e2 < 5) AND ((pm1.g1.e2 < 5) OR (pm1.g1.e1 <> '1'))"; //$NON-NLS-1$ + + helpTestRewriteCriteria(original, expected); + } + + @Test public void testRewriteNullHandling5() { + String original = "not(pm1.g1.e1 not like '%' and pm1.g1.e3 = '1') or pm1.g1.e2 < 5"; //$NON-NLS-1$ + String expected = "(pm1.g1.e1 IS NOT NULL) OR (pm1.g1.e3 <> TRUE) OR (pm1.g1.e2 < 5)"; //$NON-NLS-1$ + + helpTestRewriteCriteria(original, expected); + } + } Modified: trunk/metadata/src/main/java/org/teiid/connector/metadata/MetadataResultsPostProcessor.java =================================================================== --- trunk/metadata/src/main/java/org/teiid/connector/metadata/MetadataResultsPostProcessor.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/metadata/src/main/java/org/teiid/connector/metadata/MetadataResultsPostProcessor.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -344,13 +344,10 @@ if(!CharOperation.match(literalString.toCharArray(), valueString.toCharArray(), !literalCriteria.hasFieldWithCaseFunctions())) { criteriaPassed = false; } - } else { - // non string just check if they equal - if(value != null && !value.equals(evaluatedLiteral)) { - criteriaPassed = false; - } else if(value == null && evaluatedLiteral != null) { - criteriaPassed = false; - } + } else if(value == null || evaluatedLiteral == null) { + criteriaPassed = false; + } else if (((Comparable)value).compareTo(evaluatedLiteral) != 0) { + criteriaPassed = false; } // post processing literal criteria } Modified: trunk/metadata/src/main/java/org/teiid/metadata/index/CharOperation.java =================================================================== --- trunk/metadata/src/main/java/org/teiid/metadata/index/CharOperation.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/metadata/src/main/java/org/teiid/metadata/index/CharOperation.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -89,11 +89,6 @@ int iPattern = 0; int iName = 0; - if (patternEnd < 0) - patternEnd = pattern.length; - if (nameEnd < 0) - nameEnd = name.length; - /* check first segment */ char patternChar = 0; while ((iPattern < patternEnd) @@ -114,6 +109,9 @@ /* check sequence of star+segment */ int segmentStart; if (patternChar == '*') { + if (patternEnd == 1) { + return true; + } segmentStart = ++iPattern; // skip star } else { segmentStart = 0; // force iName check Modified: trunk/metadata/src/test/java/com/metamatrix/connector/metadata/index/TestMetadataResultsPostProcessor.java =================================================================== --- trunk/metadata/src/test/java/com/metamatrix/connector/metadata/index/TestMetadataResultsPostProcessor.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/metadata/src/test/java/com/metamatrix/connector/metadata/index/TestMetadataResultsPostProcessor.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -112,7 +112,7 @@ assertNotNull(filteredRecord); } - public void testFilterNullMatch() { + public void testFilterNullNotEqualNull() { String uuid = null; MetadataLiteralCriteria literalcriteria = new MetadataLiteralCriteria(AbstractMetadataRecord.MetadataFieldNames.UUID_FIELD, uuid); @@ -126,7 +126,7 @@ MetadataResultsPostProcessor processor = helpGetProcessor(); Object filteredRecord = processor.filterBySearchCriteria(columnRecord, criteria); - assertNotNull(filteredRecord); + assertNull(filteredRecord); } public void testFilterNullMisMatch() { Modified: trunk/test-integration/src/test/java/org/teiid/connector/language/TestLanguageUtil.java =================================================================== --- trunk/test-integration/src/test/java/org/teiid/connector/language/TestLanguageUtil.java 2009-08-19 21:08:00 UTC (rev 1265) +++ trunk/test-integration/src/test/java/org/teiid/connector/language/TestLanguageUtil.java 2009-08-20 13:14:46 UTC (rev 1266) @@ -95,7 +95,7 @@ public void testSeparateCrit_NOT() throws Exception { helpTestSeparateByAnd("((NOT (intkey = 1 AND intkey = 2)) AND (intkey = 3) AND (intkey = 4))", //$NON-NLS-1$ - new String[] { "NOT ((SmallA.IntKey = 1) AND (SmallA.IntKey = 2))", //$NON-NLS-1$ + new String[] { "(SmallA.IntKey <> 1) OR (SmallA.IntKey <> 2)", //$NON-NLS-1$ "SmallA.IntKey = 3", //$NON-NLS-1$ "SmallA.IntKey = 4" }); //$NON-NLS-1$ }