Author: shawkins Date: 2009-08-06 22:33:46 -0400 (Thu, 06 Aug 2009) New Revision: 1219 Removed: trunk/engine/src/main/java/com/metamatrix/query/validator/ValueValidationVisitor.java Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/symbol/Reference.java trunk/engine/src/main/java/com/metamatrix/query/validator/ValidationVisitor.java trunk/engine/src/main/java/org/teiid/dqp/internal/process/PreparedStatementRequest.java trunk/engine/src/main/java/org/teiid/dqp/internal/process/Request.java trunk/engine/src/test/java/com/metamatrix/query/validator/TestValidator.java trunk/engine/src/test/java/org/teiid/dqp/internal/process/TestPreparedStatement.java Log: TEIID-496 removing valuevalidationvisitor and instead adding constraints to references Modified: trunk/engine/src/main/java/com/metamatrix/query/sql/symbol/Reference.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/sql/symbol/Reference.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/main/java/com/metamatrix/query/sql/symbol/Reference.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -22,6 +22,7 @@ package com.metamatrix.query.sql.symbol; +import com.metamatrix.api.exception.query.QueryValidatorException; import com.metamatrix.core.util.Assertion; import com.metamatrix.query.metadata.TempMetadataID; import com.metamatrix.query.sql.LanguageVisitor; @@ -34,13 +35,19 @@ */ public class Reference implements Expression, ContextReference { + public interface Constraint { + public void validate(Object value) throws QueryValidatorException; + } + private boolean positional; private int refIndex; private Class<?> type; private ElementSymbol expression; - + + private Constraint constraint; + /** * Constructor for a positional Reference. */ @@ -49,6 +56,14 @@ this.positional = true; } + public Constraint getConstraint() { + return constraint; + } + + public void setConstraint(Constraint constraint) { + this.constraint = constraint; + } + /** * Constructor for an element Reference. */ @@ -64,6 +79,7 @@ if (ref.expression != null) { this.expression = (ElementSymbol)ref.expression.clone(); } + this.constraint = ref.constraint; } public boolean isResolved() { Modified: trunk/engine/src/main/java/com/metamatrix/query/validator/ValidationVisitor.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/validator/ValidationVisitor.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/main/java/com/metamatrix/query/validator/ValidationVisitor.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -35,6 +35,7 @@ import com.metamatrix.api.exception.MetaMatrixProcessingException; import com.metamatrix.api.exception.query.ExpressionEvaluationException; import com.metamatrix.api.exception.query.QueryMetadataException; +import com.metamatrix.api.exception.query.QueryValidatorException; import com.metamatrix.common.types.DataTypeManager; import com.metamatrix.query.QueryPlugin; import com.metamatrix.query.eval.Evaluator; @@ -58,6 +59,7 @@ import com.metamatrix.query.sql.lang.Insert; import com.metamatrix.query.sql.lang.Into; import com.metamatrix.query.sql.lang.IsNullCriteria; +import com.metamatrix.query.sql.lang.Limit; import com.metamatrix.query.sql.lang.MatchCriteria; import com.metamatrix.query.sql.lang.NotCriteria; import com.metamatrix.query.sql.lang.Option; @@ -90,6 +92,7 @@ import com.metamatrix.query.sql.symbol.ExpressionSymbol; import com.metamatrix.query.sql.symbol.Function; import com.metamatrix.query.sql.symbol.GroupSymbol; +import com.metamatrix.query.sql.symbol.Reference; import com.metamatrix.query.sql.symbol.SingleElementSymbol; import com.metamatrix.query.sql.util.SymbolMap; import com.metamatrix.query.sql.visitor.AggregateSymbolCollectorVisitor; @@ -104,7 +107,24 @@ public class ValidationVisitor extends AbstractValidationVisitor { - // State during validation + private final class PositiveIntegerConstraint implements + Reference.Constraint { + + private String msgKey; + + public PositiveIntegerConstraint(String msgKey) { + this.msgKey = msgKey; + } + + @Override + public void validate(Object value) throws QueryValidatorException { + if (((Integer)value).intValue() < 0) { + throw new QueryValidatorException(QueryPlugin.Util.getString(msgKey)); //$NON-NLS-1$ + } + } + } + + // State during validation private boolean isXML = false; // only used for Query commands // update procedure being validated @@ -1033,6 +1053,78 @@ if (isNonComparable(obj.getLeftExpression())) { handleValidationError(QueryPlugin.Util.getString(ErrorMessageKeys.VALIDATOR_0027, obj),obj); } + + // Validate use of 'rowlimit' and 'rowlimitexception' pseudo-functions - they cannot be nested within another + // function, and their operands must be a nonnegative integers + + // Collect all occurrances of rowlimit function + List rowLimitFunctions = new ArrayList(); + FunctionCollectorVisitor visitor = new FunctionCollectorVisitor(rowLimitFunctions, FunctionLibrary.ROWLIMIT); + PreOrderNavigator.doVisit(obj, visitor); + visitor = new FunctionCollectorVisitor(rowLimitFunctions, FunctionLibrary.ROWLIMITEXCEPTION); + PreOrderNavigator.doVisit(obj, visitor); + final int functionCount = rowLimitFunctions.size(); + if (functionCount > 0) { + Function function = null; + Expression expr = null; + if (obj.getLeftExpression() instanceof Function) { + Function leftExpr = (Function)obj.getLeftExpression(); + if (leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || + leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { + function = leftExpr; + expr = obj.getRightExpression(); + } + } + if (function == null && obj.getRightExpression() instanceof Function) { + Function rightExpr = (Function)obj.getRightExpression(); + if (rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || + rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { + function = rightExpr; + expr = obj.getLeftExpression(); + } + } + if (function == null) { + // must be nested, which is invalid + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.0"), obj); //$NON-NLS-1$ + } else { + if (expr instanceof Constant) { + Constant constant = (Constant)expr; + if (constant.getValue() instanceof Integer) { + Integer integer = (Integer)constant.getValue(); + if (integer.intValue() < 0) { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ + } + } else { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ + } + } else if (expr instanceof Reference) { + ((Reference)expr).setConstraint(new PositiveIntegerConstraint("ValidationVisitor.1")); //$NON-NLS-1$ + } else { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ + } + } + } } + + public void visit(Limit obj) { + Expression offsetExpr = obj.getOffset(); + if (offsetExpr instanceof Constant) { + Integer offset = (Integer)((Constant)offsetExpr).getValue(); + if (offset.intValue() < 0) { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badoffset2"), obj); //$NON-NLS-1$ + } + } else if (offsetExpr instanceof Reference) { + ((Reference)offsetExpr).setConstraint(new PositiveIntegerConstraint("ValidationVisitor.badoffset2")); //$NON-NLS-1$ + } + Expression limitExpr = obj.getRowLimit(); + if (limitExpr instanceof Constant) { + Integer limit = (Integer)((Constant)limitExpr).getValue(); + if (limit.intValue() < 0) { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badlimit2"), obj); //$NON-NLS-1$ + } + } else if (limitExpr instanceof Reference) { + ((Reference)limitExpr).setConstraint(new PositiveIntegerConstraint("ValidationVisitor.badlimit2")); //$NON-NLS-1$ + } + } } Deleted: trunk/engine/src/main/java/com/metamatrix/query/validator/ValueValidationVisitor.java =================================================================== --- trunk/engine/src/main/java/com/metamatrix/query/validator/ValueValidationVisitor.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/main/java/com/metamatrix/query/validator/ValueValidationVisitor.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -1,122 +0,0 @@ -/* - * JBoss, Home of Professional Open Source. - * See the COPYRIGHT.txt file distributed with this work for information - * regarding copyright ownership. Some portions may be licensed - * to Red Hat, Inc. under one or more contributor license agreements. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA - * 02110-1301 USA. - */ - -package com.metamatrix.query.validator; - -import java.util.ArrayList; -import java.util.List; - -import com.metamatrix.query.QueryPlugin; -import com.metamatrix.query.function.FunctionLibrary; -import com.metamatrix.query.sql.lang.CompareCriteria; -import com.metamatrix.query.sql.lang.Limit; -import com.metamatrix.query.sql.navigator.PreOrderNavigator; -import com.metamatrix.query.sql.symbol.Constant; -import com.metamatrix.query.sql.symbol.Expression; -import com.metamatrix.query.sql.symbol.Function; -import com.metamatrix.query.sql.symbol.Reference; -import com.metamatrix.query.sql.visitor.FunctionCollectorVisitor; - -public class ValueValidationVisitor extends AbstractValidationVisitor { - - public void visit(CompareCriteria obj) { - // Validate use of 'rowlimit' and 'rowlimitexception' pseudo-functions - they cannot be nested within another - // function, and their operands must be a nonnegative integers - - // Collect all occurrances of rowlimit function - List rowLimitFunctions = new ArrayList(); - FunctionCollectorVisitor visitor = new FunctionCollectorVisitor(rowLimitFunctions, FunctionLibrary.ROWLIMIT); - PreOrderNavigator.doVisit(obj, visitor); - visitor = new FunctionCollectorVisitor(rowLimitFunctions, FunctionLibrary.ROWLIMITEXCEPTION); - PreOrderNavigator.doVisit(obj, visitor); - final int functionCount = rowLimitFunctions.size(); - if (functionCount > 0) { - Function function = null; - Expression expr = null; - if (obj.getLeftExpression() instanceof Function) { - Function leftExpr = (Function)obj.getLeftExpression(); - if (leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || - leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { - function = leftExpr; - expr = obj.getRightExpression(); - } - } - if (function == null && obj.getRightExpression() instanceof Function) { - Function rightExpr = (Function)obj.getRightExpression(); - if (rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || - rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { - function = rightExpr; - expr = obj.getLeftExpression(); - } - } - if (function == null) { - // must be nested, which is invalid - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.0"), obj); //$NON-NLS-1$ - } else { - if (expr instanceof Reference) { - expr = ((Reference)expr).getExpression(); - } - if (expr instanceof Constant) { - Constant constant = (Constant)expr; - if (constant.getValue() instanceof Integer) { - Integer integer = (Integer)constant.getValue(); - if (integer.intValue() < 0) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ - } - } else { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ - } - } else { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.1"), obj); //$NON-NLS-1$ - } - } - } - } - - public void visit(Limit obj) { - Expression offsetExpr = obj.getOffset(); - if (offsetExpr instanceof Reference) { - offsetExpr = ((Reference)offsetExpr).getExpression(); - } - if (offsetExpr instanceof Constant) { - Integer offset = (Integer)((Constant)offsetExpr).getValue(); - if (offset == null) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badoffset1"), obj); //$NON-NLS-1$ - } else if (offset.intValue() < 0) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badoffset2"), obj); //$NON-NLS-1$ - } - } - Expression limitExpr = obj.getRowLimit(); - if (limitExpr instanceof Reference) { - limitExpr = ((Reference)limitExpr).getExpression(); - } - if (limitExpr instanceof Constant) { - Integer limit = (Integer)((Constant)limitExpr).getValue(); - if (limit == null) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badlimit1"), obj); //$NON-NLS-1$ - } else if (limit.intValue() < 0) { - handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.badlimit2"), obj); //$NON-NLS-1$ - } - } - } - -} Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/process/PreparedStatementRequest.java =================================================================== --- trunk/engine/src/main/java/org/teiid/dqp/internal/process/PreparedStatementRequest.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/main/java/org/teiid/dqp/internal/process/PreparedStatementRequest.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -123,12 +123,6 @@ } } - @Override - protected void validateQueryValues(Command command) - throws QueryValidatorException, MetaMatrixComponentException { - //do nothing initially - check after parameter values have been set - } - /** * @throws MetaMatrixComponentException * @throws QueryValidatorException @@ -174,7 +168,7 @@ List<Reference> params = prepPlan.getReferences(); List<?> values = requestMsg.getParameterValues(); - resolveAndValidateParameters(this.userCommand, params, values); + PreparedStatementRequest.resolveParameterValues(params, values, this.context); } return prepPlan.getRewritenCommand(); } @@ -213,7 +207,7 @@ List<VariableContext> contexts = new LinkedList<VariableContext>(); List<List<Object>> multiValues = new ArrayList<List<Object>>(this.prepPlan.getReferences().size()); for (List<?> values : paramValues) { - resolveAndValidateParameters(this.userCommand, this.prepPlan.getReferences(), values); + PreparedStatementRequest.resolveParameterValues(this.prepPlan.getReferences(), values, this.context); contexts.add(this.context.getVariableContext()); if(supportPreparedBatchUpdate){ if (multiValues.isEmpty()) { @@ -259,22 +253,14 @@ this.processPlan = planner.optimize(ctn, idGenerator, metadata, capabilitiesFinder, analysisRecord, context); } - private void resolveAndValidateParameters(Command command, List<Reference> params, - List<?> values) throws QueryResolverException, - MetaMatrixComponentException, QueryValidatorException { - // validate parameters values - right number and right type - PreparedStatementRequest.resolveParameterValues(params, values, this.context); - // call back to Request.validateQueryValues to ensure that bound references are valid - super.validateQueryValues(command); - } - /** * @param params * @param values * @throws QueryResolverException + * @throws QueryValidatorException */ public static void resolveParameterValues(List<Reference> params, - List values, CommandContext context) throws QueryResolverException, MetaMatrixComponentException { + List values, CommandContext context) throws QueryResolverException, MetaMatrixComponentException, QueryValidatorException { VariableContext result = new VariableContext(); //the size of the values must be the same as that of the parameters if (params.size() != values.size()) { @@ -303,6 +289,9 @@ } } + if (param.getConstraint() != null) { + param.getConstraint().validate(value); + } //bind variable result.setGlobalValue(param.getContextSymbol(), value); } Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/process/Request.java =================================================================== --- trunk/engine/src/main/java/org/teiid/dqp/internal/process/Request.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/main/java/org/teiid/dqp/internal/process/Request.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -105,7 +105,6 @@ import com.metamatrix.query.validator.Validator; import com.metamatrix.query.validator.ValidatorFailure; import com.metamatrix.query.validator.ValidatorReport; -import com.metamatrix.query.validator.ValueValidationVisitor; /** * Server side representation of the RequestMessage. Knows how to process itself. @@ -326,13 +325,6 @@ } } - protected void validateQueryValues(Command command) - throws QueryValidatorException, MetaMatrixComponentException { - - AbstractValidationVisitor visitor = new ValueValidationVisitor(); - validateWithVisitor(visitor, metadata, command, false); - } - private Command parseCommand() throws QueryParserException { String[] commands = requestMsg.getCommands(); ParseInfo parseInfo = createParseInfo(this.requestMsg); @@ -471,8 +463,6 @@ validateQuery(command, true); - validateQueryValues(command); - command = QueryRewriter.rewrite(command, null, metadata, context); /* @@ -615,8 +605,6 @@ validateQuery(newCommand, isRootXQuery); - validateQueryValues(newCommand); - if (isRootXQuery) { validateEntitlement(newCommand); } Modified: trunk/engine/src/test/java/com/metamatrix/query/validator/TestValidator.java =================================================================== --- trunk/engine/src/test/java/com/metamatrix/query/validator/TestValidator.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/test/java/com/metamatrix/query/validator/TestValidator.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -354,12 +354,9 @@ try { ValidatorReport report = Validator.validate(command, metadata); - ValidatorReport report3 = Validator.validate(command, metadata, new ValueValidationVisitor(), true); - // Get invalid objects from report Collection actualObjs = new ArrayList(); report.collectInvalidObjects(actualObjs); - report3.collectInvalidObjects(actualObjs); // Compare expected and actual objects Set expectedStrings = new HashSet(Arrays.asList(expectedStringArray)); @@ -371,7 +368,7 @@ } if(expectedStrings.size() == 0 && actualStrings.size() > 0) { - fail("Expected no failures but got some: " + report.getFailureMessage() + ", " + report3.getFailureMessage()); //$NON-NLS-1$ //$NON-NLS-2$ + fail("Expected no failures but got some: " + report.getFailureMessage()); //$NON-NLS-1$ } else if(actualStrings.size() == 0 && expectedStrings.size() > 0) { fail("Expected some failures but got none for sql = " + command); //$NON-NLS-1$ } else { Modified: trunk/engine/src/test/java/org/teiid/dqp/internal/process/TestPreparedStatement.java =================================================================== --- trunk/engine/src/test/java/org/teiid/dqp/internal/process/TestPreparedStatement.java 2009-08-06 20:11:11 UTC (rev 1218) +++ trunk/engine/src/test/java/org/teiid/dqp/internal/process/TestPreparedStatement.java 2009-08-07 02:33:46 UTC (rev 1219) @@ -391,5 +391,13 @@ TestProcessor.sampleData2b(dataManager); helpTestProcessing(preparedSql, values, expected, dataManager, FakeMetadataFactory.example1Cached(), false, false); } + + @Test(expected=QueryValidatorException.class) public void testLimitValidation() throws Exception { + String preparedSql = "select pm1.g1.e1 from pm1.g1 limit ?"; //$NON-NLS-1$ + + List values = Arrays.asList(-1); + FakeDataManager dataManager = new FakeDataManager(); + helpTestProcessing(preparedSql, values, null, dataManager, FakeMetadataFactory.example1Cached(), false, false); + } }