Author: shawkins Date: 2010-10-07 14:36:29 -0400 (Thu, 07 Oct 2010) New Revision: 2634 Modified: branches/7.1.x/build/kits/jboss-container/teiid-releasenotes.html branches/7.1.x/engine/src/main/java/org/teiid/dqp/internal/process/Request.java branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/command/UpdateProcedureResolver.java branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Command.java branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/DynamicCommand.java branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Query.java branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/QueryCommand.java branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java branches/7.1.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java Log: TEIID-1293 TEIID-1294 tightening validation related to updates. Modified: branches/7.1.x/build/kits/jboss-container/teiid-releasenotes.html =================================================================== --- branches/7.1.x/build/kits/jboss-container/teiid-releasenotes.html 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/build/kits/jboss-container/teiid-releasenotes.html 2010-10-07 18:36:29 UTC (rev 2634) @@ -43,6 +43,8 @@ </ul> <h4>from 7.1</h4> <ul> + <li>Subqueries are no longer allowed to be SELECT INTO. + <li>INSERT/UPDATE/DELETE cannot be used to create implicit return cursors in non-update virtual procedures. You can instead use "integer_var = UPDATE ...; SELECT integer_var;". <li>The SYSADMIN schema was created to hold procedures and tables that should not be generally accessible. SYS and pg_catalog are now always accessible - permissions do not apply to these schemas. The SYS.getBinaryVDBResource, SYS.getCharacterVDBResource, and SYS.getVDBResourcePaths have been replaced with the SYSADMIN.VDBResources table. The Matviews table and the refreshMatView/refreshMatViewRow procedures were also moved into SYSADMIN. <li>Overwriting an existing VDB will cause old connections to be terminated. Production systems should rely on VDB versioning. Modified: branches/7.1.x/engine/src/main/java/org/teiid/dqp/internal/process/Request.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/dqp/internal/process/Request.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/dqp/internal/process/Request.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -78,9 +78,7 @@ import org.teiid.query.sql.lang.BatchedUpdateCommand; import org.teiid.query.sql.lang.Command; import org.teiid.query.sql.lang.Limit; -import org.teiid.query.sql.lang.Query; import org.teiid.query.sql.lang.QueryCommand; -import org.teiid.query.sql.lang.SetQuery; import org.teiid.query.sql.lang.StoredProcedure; import org.teiid.query.sql.symbol.Constant; import org.teiid.query.sql.symbol.GroupSymbol; @@ -203,20 +201,8 @@ } protected void createCommandContext() throws QueryValidatorException { - boolean returnsResultSet = false; - this.returnsUpdateCount = true; - if(userCommand instanceof Query) { - Query query = (Query)userCommand; - returnsResultSet = query.getInto() == null; - returnsUpdateCount = !returnsResultSet; - } else if (userCommand instanceof SetQuery) { - returnsResultSet = true; - returnsUpdateCount = false; - } else if (userCommand instanceof StoredProcedure) { - returnsUpdateCount = false; - StoredProcedure proc = (StoredProcedure)userCommand; - returnsResultSet = proc.returnsResultSet(); - } + boolean returnsResultSet = userCommand.returnsResultSet(); + this.returnsUpdateCount = !(userCommand instanceof StoredProcedure) && !returnsResultSet; if ((this.requestMsg.getResultsMode() == ResultsMode.UPDATECOUNT && !returnsUpdateCount) || (this.requestMsg.getResultsMode() == ResultsMode.RESULTSET && !returnsResultSet)) { throw new QueryValidatorException(QueryPlugin.Util.getString(this.requestMsg.getResultsMode()==ResultsMode.RESULTSET?"Request.no_result_set":"Request.result_set")); //$NON-NLS-1$ //$NON-NLS-2$ Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/command/UpdateProcedureResolver.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/command/UpdateProcedureResolver.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/command/UpdateProcedureResolver.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -231,9 +231,12 @@ dynamicCommand.setAsColumns(Collections.EMPTY_LIST); } } - //this could be the last select statement, set the projected symbol - //on the virtual procedure command - command.setResultsCommand(subCommand); + + if (subCommand.returnsResultSet()) { + //this could be the last select statement, set the projected symbol + //on the virtual procedure command + command.setResultsCommand(subCommand); + } } break; Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Command.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Command.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Command.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -340,4 +340,8 @@ return EquivalenceUtil.areEqual(this.cacheHint, cmd.cacheHint) && EquivalenceUtil.areEqual(this.option, cmd.option); } + + public boolean returnsResultSet() { + return false; + } } Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/DynamicCommand.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/DynamicCommand.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/DynamicCommand.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -255,5 +255,10 @@ public void setAsClauseSet(boolean asClauseSet) { this.asClauseSet = asClauseSet; } + + @Override + public boolean returnsResultSet() { + return intoGroup == null; + } } Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Query.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Query.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/Query.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -441,4 +441,9 @@ public Query getProjectedQuery() { return this; } + + @Override + public boolean returnsResultSet() { + return into == null; + } } // END CLASS Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/QueryCommand.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/QueryCommand.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/sql/lang/QueryCommand.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -62,4 +62,9 @@ } public abstract Query getProjectedQuery(); + + @Override + public boolean returnsResultSet() { + return true; + } } 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-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -63,6 +63,7 @@ import org.teiid.query.sql.lang.DependentSetCriteria; import org.teiid.query.sql.lang.Drop; import org.teiid.query.sql.lang.DynamicCommand; +import org.teiid.query.sql.lang.ExistsCriteria; import org.teiid.query.sql.lang.GroupBy; import org.teiid.query.sql.lang.Insert; import org.teiid.query.sql.lang.Into; @@ -81,6 +82,8 @@ import org.teiid.query.sql.lang.SetCriteria; import org.teiid.query.sql.lang.SetQuery; import org.teiid.query.sql.lang.SubqueryCompareCriteria; +import org.teiid.query.sql.lang.SubqueryContainer; +import org.teiid.query.sql.lang.SubqueryFromClause; import org.teiid.query.sql.lang.SubquerySetCriteria; import org.teiid.query.sql.lang.TextTable; import org.teiid.query.sql.lang.Update; @@ -266,6 +269,7 @@ } public void visit(SubquerySetCriteria obj) { + validateSubquery(obj); if (isNonComparable(obj.getExpression())) { handleValidationError(QueryPlugin.Util.getString("ERR.015.012.0027", obj),obj); //$NON-NLS-1$ } @@ -376,6 +380,7 @@ @Override public void visit(ScalarSubquery obj) { + validateSubquery(obj); Collection<SingleElementSymbol> projSymbols = obj.getCommand().getProjectedSymbols(); //Scalar subquery should have one projected symbol (query with one expression @@ -1021,6 +1026,7 @@ * @since 4.3 */ public void visit(SubqueryCompareCriteria obj) { + validateSubquery(obj); if (isNonComparable(obj.getLeftExpression())) { handleValidationError(QueryPlugin.Util.getString("ERR.015.012.0027", obj),obj); //$NON-NLS-1$ } @@ -1349,5 +1355,21 @@ handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.xmlserialize_type"), obj); //$NON-NLS-1$ } } - + + @Override + public void visit(ExistsCriteria obj) { + validateSubquery(obj); + } + + @Override + public void visit(SubqueryFromClause obj) { + validateSubquery(obj); + } + + public void validateSubquery(SubqueryContainer subQuery) { + if (subQuery.getCommand() instanceof Query && ((Query)subQuery.getCommand()).getInto() != null) { + handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.subquery_insert"), subQuery.getCommand()); //$NON-NLS-1$ + } + } + } Modified: branches/7.1.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java =================================================================== --- branches/7.1.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java 2010-10-07 18:24:52 UTC (rev 2633) +++ branches/7.1.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java 2010-10-07 18:36:29 UTC (rev 2634) @@ -2029,5 +2029,20 @@ @Test public void testValidateScalarSubqueryTooManyColumns() { helpValidate("SELECT e2, (SELECT e1, e2 FROM pm1.g1 WHERE e2 = '3') FROM pm1.g2", new String[] {"SELECT e1, e2 FROM pm1.g1 WHERE e2 = '3'"}, FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ //$NON-NLS-2$ } + + @Test public void testInvalidIntoSubquery() { + helpValidate("SELECT e2, (SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3') FROM pm1.g2", new String[] {"SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3'"}, FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ //$NON-NLS-2$ + } + @Test public void testInvalidIntoSubquery1() { + helpValidate("SELECT e2 FROM pm1.g2 WHERE EXISTS (SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3')", new String[] {"SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3'"}, FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ //$NON-NLS-2$ + } + + @Test public void testInvalidIntoSubquery2() { + helpValidate("SELECT * FROM (SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3') x", new String[] {"SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3'"}, FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ //$NON-NLS-2$ + } + + @Test public void testInvalidIntoSubquery3() { + helpValidate("SELECT e2 FROM pm1.g2 WHERE e2 in (SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3')", new String[] {"SELECT e1, e2 INTO #x FROM pm1.g1 WHERE e2 = '3'"}, FakeMetadataFactory.example1Cached()); //$NON-NLS-1$ //$NON-NLS-2$ + } }