[teiid-commits] teiid SVN: r2770 - in branches/7.1.x: engine/src/main/java/org/teiid/query/rewriter and 3 other directories.

Author: shawkins Date: 2010-12-13 13:09:50 -0500 (Mon, 13 Dec 2010) New Revision: 2770 Modified: branches/7.1.x/documentation/reference/src/main/docbook/en-US/content/procedures.xml branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java branches/7.1.x/engine/src/main/java/org/teiid/query/sql/visitor/CriteriaTranslatorVisitor.java branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties Log: TEIID-1346 preventing translate criteria from throwing exceptions in what used to be common cases. Modified: branches/7.1.x/documentation/reference/src/main/docbook/en-US/content/procedures.xml =================================================================== --- branches/7.1.x/documentation/reference/src/main/docbook/en-US/content/procedures.xml 2010-12-13 17:55:26 UTC (rev 2769) +++ branches/7.1.x/documentation/reference/src/main/docbook/en-US/content/procedures.xml 2010-12-13 18:09:50 UTC (rev 2770) @@ -513,6 +513,7 @@ </para> </listitem> </itemizedlist> + <para>Each unoptimized conjunct of the user criteria is evaluated against the criteria selector. If any conjunct matches then HAS CRITERIA evaluates to TRUE. The use of OR or NOT will prevent contained predicates from matching the criteria selector.</para> <para>Some samples of the HAS CRITERIA clause:</para> <informaltable> <tgroup cols="2"> @@ -579,6 +580,7 @@ </para> </listitem> </itemizedlist> + <para>Each unoptimized conjunct of the user criteria is translated using the criteria selector. If a conjunct does not match the selector, it will not be translated - which effectively treats the conjunct as TRUE. The use of OR or NOT will prevent contained predicates from matching the criteria selector.</para> <para>Some samples of TRANSLATE CRITERIA:</para> <informaltable> <tgroup cols="2"> @@ -606,7 +608,6 @@ </tbody> </tgroup> </informaltable> - <note><para>If a specific predicate type or column set is specified by TRANSALATE CRITERIA, but the user criteria has predicates that do not match, then an exception will be thrown.</para></note> </section> </section> <section> Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2010-12-13 17:55:26 UTC (rev 2769) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2010-12-13 18:09:50 UTC (rev 2770) @@ -146,7 +146,6 @@ import org.teiid.query.sql.visitor.ElementCollectorVisitor; import org.teiid.query.sql.visitor.EvaluatableVisitor; import org.teiid.query.sql.visitor.ExpressionMappingVisitor; -import org.teiid.query.sql.visitor.PredicateCollectorVisitor; import org.teiid.query.sql.visitor.EvaluatableVisitor.EvaluationLevel; import org.teiid.query.util.CommandContext; import org.teiid.translator.SourceSystemFunctions; @@ -466,9 +465,7 @@ } // collect all predicate criteria present on the user's criteria - Iterator criteriaIter = PredicateCollectorVisitor.getPredicates(userCrit).iterator(); - while(criteriaIter.hasNext()) { - Criteria predicateCriteria = (Criteria) criteriaIter.next(); + for (Criteria predicateCriteria : Criteria.separateCriteriaByAnd(userCrit)) { // atleast one of the hasElemnets should be on this predicate else // proceed to the next predicate Collection<ElementSymbol> predElmnts = ElementCollectorVisitor.getElements(predicateCriteria, true); @@ -562,7 +559,7 @@ // create a clone of user's criteria that is then translated Criteria userClone = (Criteria) userCriteria.clone(); - translateVisitor.translate(userClone); + userClone = translateVisitor.translate(userClone); // translated criteria ((TranslatableProcedureContainer)userCmd).addImplicitParameters(translateVisitor.getImplicitParams()); Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/sql/visitor/CriteriaTranslatorVisitor.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/sql/visitor/CriteriaTranslatorVisitor.java 2010-12-13 17:55:26 UTC (rev 2769) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/sql/visitor/CriteriaTranslatorVisitor.java 2010-12-13 18:09:50 UTC (rev 2770) @@ -25,20 +25,20 @@ import java.util.Collection; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedList; import java.util.Map; -import org.teiid.api.exception.query.QueryValidatorException; -import org.teiid.core.TeiidRuntimeException; +import net.sf.saxon.query.QueryReader; + import org.teiid.core.util.Assertion; -import org.teiid.query.QueryPlugin; +import org.teiid.query.rewriter.QueryRewriter; +import org.teiid.query.sql.lang.AbstractSetCriteria; import org.teiid.query.sql.lang.BetweenCriteria; import org.teiid.query.sql.lang.CompareCriteria; import org.teiid.query.sql.lang.Criteria; -import org.teiid.query.sql.lang.DependentSetCriteria; import org.teiid.query.sql.lang.IsNullCriteria; import org.teiid.query.sql.lang.MatchCriteria; -import org.teiid.query.sql.lang.SetCriteria; -import org.teiid.query.sql.navigator.PreOrderNavigator; +import org.teiid.query.sql.navigator.DeepPostOrderNavigator; import org.teiid.query.sql.proc.CriteriaSelector; import org.teiid.query.sql.symbol.ElementSymbol; import org.teiid.query.sql.symbol.Expression; @@ -49,106 +49,11 @@ * <p> This class is used to translate criteria specified on the user's update command against * the virtual group, the elements on this criteria are replaced by elements on the query * transformation that defines the virtual group. Parts of the criteria are selectively translated - * if a CriteriaSelector is specified, also if the user explicty defines translations for some + * if a CriteriaSelector is specified, also if the user explicitly defines translations for some * of the elements those translations override any symbol mappings.</p> */ public class CriteriaTranslatorVisitor extends ExpressionMappingVisitor { - class CriteriaTranslatorNavigator extends PreOrderNavigator { - - public CriteriaTranslatorNavigator() { - super(CriteriaTranslatorVisitor.this); - } - - /** - * <p> This method updates the <code>BetweenCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions.</p> - * @param obj The BetweenCriteria object to be updated with translated expressions - */ - public void visit(BetweenCriteria obj) { - if (!selectorContainsCriteriaElements(obj, CriteriaSelector.BETWEEN)) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - super.visit(obj); - } - - /** - * <p> This method updates the <code>CompareCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions.</p> - * @param obj The CompareCriteria object to be updated with translated expressions - */ - public void visit(CompareCriteria obj) { - - if (!selectorContainsCriteriaElements(obj, obj.getOperator())) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - - super.visit(obj); - } - - /** - * <p> This method updates the <code>IsNullCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions.</p> - * @param obj The IsNullCriteria object to be updated with translated expressions - */ - public void visit(IsNullCriteria obj) { - - if (!selectorContainsCriteriaElements(obj, CriteriaSelector.IS_NULL)) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - super.visit(obj); - } - - /** - * <p> This method updates the <code>MatchCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions</p> - * @param obj The SetCriteria object to be updated with translated expressions - */ - public void visit(MatchCriteria obj) { - - if (!selectorContainsCriteriaElements(obj, CriteriaSelector.LIKE)) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - - super.visit(obj); - } - - /** - * <p> This method updates the <code>SetCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions</p> - * @param obj The SetCriteria object to be updated with translated expressions - */ - public void visit(SetCriteria obj) { - - if (!selectorContainsCriteriaElements(obj, CriteriaSelector.IN)) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - - super.visit(obj); - } - - /** - * <p> This method updates the <code>SetCriteria</code> object it receives as an - * argument by replacing the virtual elements present in the expressions in the - * function with translated expressions</p> - * @param obj The SetCriteria object to be updated with translated expressions - */ - public void visit(DependentSetCriteria obj) { - - if (!selectorContainsCriteriaElements(obj, CriteriaSelector.IN)) { - throw new TeiidRuntimeException(new QueryValidatorException(QueryPlugin.Util.getString("Translate.error", obj, selector))); //$NON-NLS-1$ - } - - super.visit(obj); - } - - } - // criteria selector specified on the TranslateCriteria obj private CriteriaSelector selector; @@ -158,13 +63,6 @@ private Map<ElementSymbol, Reference> implicitParams = new HashMap<ElementSymbol, Reference>(); /** - * <p> This constructor initialises the visitor</p> - */ - public CriteriaTranslatorVisitor() { - this(null); - } - - /** * <p> This constructor initializes this object by setting the symbolMap.</p> * @param symbolMap A map of virtual elements to their counterparts in transform * defining the virtual group @@ -251,13 +149,39 @@ return implicitParams; } - public void translate(Criteria crit) throws QueryValidatorException { - CriteriaTranslatorNavigator nav = new CriteriaTranslatorNavigator(); - try { - crit.acceptVisitor(nav); - } catch (TeiidRuntimeException e) { - throw (QueryValidatorException)e.getCause(); + public Criteria translate(Criteria crit) { + LinkedList<Criteria> crits = new LinkedList<Criteria>(); + for (Criteria conjunct : Criteria.separateCriteriaByAnd(crit)) { + if (conjunct instanceof BetweenCriteria) { + if (!selectorContainsCriteriaElements(conjunct, CriteriaSelector.BETWEEN)) { + continue; + } + } else if (conjunct instanceof CompareCriteria) { + if (!selectorContainsCriteriaElements(conjunct, ((CompareCriteria)conjunct).getOperator())) { + continue; + } + } else if (conjunct instanceof IsNullCriteria) { + if (!selectorContainsCriteriaElements(conjunct, CriteriaSelector.IS_NULL)) { + continue; + } + } else if (conjunct instanceof MatchCriteria) { + if (!selectorContainsCriteriaElements(conjunct, CriteriaSelector.LIKE)) { + continue; + } + } else if (conjunct instanceof AbstractSetCriteria) { + if (!selectorContainsCriteriaElements(conjunct, CriteriaSelector.IN)) { + continue; + } + } else if (!selectorContainsCriteriaElements(conjunct, CriteriaSelector.NO_TYPE)) { + continue; + } + DeepPostOrderNavigator.doVisit(conjunct, this); + crits.add(conjunct); + } + if (crits.isEmpty()) { + return QueryRewriter.TRUE_CRITERIA; } + return Criteria.combineCriteria(crits); } } Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2010-12-13 17:55:26 UTC (rev 2769) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2010-12-13 18:09:50 UTC (rev 2770) @@ -39,7 +39,6 @@ import org.teiid.api.exception.query.QueryMetadataException; import org.teiid.api.exception.query.QueryValidatorException; import org.teiid.core.TeiidComponentException; -import org.teiid.core.TeiidException; import org.teiid.core.TeiidProcessingException; import org.teiid.core.types.DataTypeManager; import org.teiid.core.util.EquivalenceUtil; @@ -127,6 +126,7 @@ import org.teiid.query.sql.visitor.EvaluatableVisitor; import org.teiid.query.sql.visitor.FunctionCollectorVisitor; import org.teiid.query.sql.visitor.GroupCollectorVisitor; +import org.teiid.query.sql.visitor.GroupsUsedByElementsVisitor; import org.teiid.query.sql.visitor.PredicateCollectorVisitor; import org.teiid.query.sql.visitor.SQLStringVisitor; import org.teiid.query.sql.visitor.ValueIteratorProviderCollectorVisitor; @@ -195,25 +195,8 @@ validateHasProjectedSymbols(obj); GroupSymbol group = obj.getGroup(); validateGroupSupportsUpdate(group); - Criteria crit = obj.getCriteria(); - validateVirtualUpdate(group, crit); } - private void validateVirtualUpdate(GroupSymbol group, - Criteria crit) { - if (crit == null) { - return; - } - try { - if (getMetadata().isVirtualGroup(group.getMetadataID()) && - !ValueIteratorProviderCollectorVisitor.getValueIteratorProviders(crit).isEmpty()) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.virtual_update_subquery"), crit); //$NON-NLS-1$ - } - } catch (TeiidException e) { - handleException(e); - } - } - public void visit(GroupBy obj) { // Get list of all group by IDs List groupBySymbols = obj.getSymbols(); @@ -319,7 +302,6 @@ validateHasProjectedSymbols(obj); validateGroupSupportsUpdate(obj.getGroup()); validateUpdate(obj); - validateVirtualUpdate(obj.getGroup(), obj.getCriteria()); } public void visit(Into obj) { @@ -585,16 +567,10 @@ } Collection transleElmnts = ElementCollectorVisitor.getElements(obj, true); - Collection groups = GroupCollectorVisitor.getGroups(this.currentCommand, true); + Collection<GroupSymbol> groups = GroupCollectorVisitor.getGroups(this.currentCommand, true); int selectType = obj.getSelector().getSelectorType(); - Collection predicates = PredicateCollectorVisitor.getPredicates(userCrit); - if (predicates.size() != Criteria.separateCriteriaByAnd(userCrit).size()) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.translated_or"), userCrit); //$NON-NLS-1$ - } - Iterator critIter = predicates.iterator(); - while(critIter.hasNext()) { - Criteria predCrit = (Criteria) critIter.next(); + for (Criteria predCrit : Criteria.separateCriteriaByAnd(userCrit)) { if(selectType != CriteriaSelector.NO_TYPE) { if(predCrit instanceof CompareCriteria) { CompareCriteria ccCrit = (CompareCriteria) predCrit; @@ -629,16 +605,7 @@ handleValidationError(QueryPlugin.Util.getString("ERR.015.012.0022", criteriaElement), criteriaElement); //$NON-NLS-1$ } - Iterator mapElmntIter = ElementCollectorVisitor.getElements(mappedExpr, true).iterator(); - boolean groupMatch = false; - while(mapElmntIter.hasNext()) { - ElementSymbol mapElement = (ElementSymbol) mapElmntIter.next(); - GroupSymbol mapGrp = mapElement.getGroupSymbol(); - if(groups.contains(mapGrp)) { - groupMatch = true; - } - } - if(!groupMatch) { + if (!groups.containsAll(GroupsUsedByElementsVisitor.getGroups(mappedExpr))) { handleValidationError(QueryPlugin.Util.getString("ERR.015.012.0023", criteriaElement), criteriaElement); //$NON-NLS-1$ } } Modified: branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties =================================================================== --- branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties 2010-12-13 17:55:26 UTC (rev 2769) +++ branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties 2010-12-13 18:09:50 UTC (rev 2770) @@ -191,9 +191,7 @@ ValidationVisitor.expression_requires_name = Non-column expressions require a name in XMLATTRIBUTES, XMLFOREST, or QUERYSTRING ValidationVisitor.invalid_lookup_key=Expressions of type OBJECT, CLOB, BLOB, or XML cannot be used as LOOKUP key columns: {0}. ValidationVisitor.limit_not_valid_for_xml=The limit clause cannot be used on an XML document query. -ValidationVisitor.translated_or=Translated user criteria must not contain OR criteria ValidationVisitor.union_insert = Select into is not allowed under a set operation: {0}. -ValidationVisitor.virtual_update_subquery = Subqueries are not allowed in the criteria for a virtual UPDATE/DELETE: {0} ERR.015.012.0029 = INSERT, UPDATE, and DELETE not allowed on XML documents ERR.015.012.0030 = Commands used in stored procedure language not allowed on XML documents ERR.015.012.0031 = Queries against XML documents can not have a GROUP By clause