You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by da...@apache.org on 2009/08/31 18:05:24 UTC
svn commit: r809643 - in /db/derby/code/trunk/java:
client/org/apache/derby/client/am/Statement.java
testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java
Author: dag
Date: Mon Aug 31 16:05:24 2009
New Revision: 809643
URL: http://svn.apache.org/viewvc?rev=809643&view=rev
Log:
DERBY-4038 Network client raises error "executeQuery method can not be used for update" when sql is preceded by /* */ comments
This patch, derby-4338-d, fixes the seen bug, and also generalizes the
method used by the client driver to find the first significant
(non-comment) SQL statement identifier, i.e. a keyword. This is needed
by the client's statement classification logic. For certain input
strings the old method also gave wrong results, cf. the new test cases
added.
Modified:
db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java
Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java?rev=809643&r1=809642&r2=809643&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java Mon Aug 31 16:05:24 2009
@@ -2319,59 +2319,21 @@
// Should investigate if it can be optimized.. if we can avoid this parsing..
//
void parseSqlAndSetSqlModes(String sql) throws SqlException {
- String delims = "\t\n\r\f=? (";
- java.util.StringTokenizer tokenizer = null;
- String firstToken = null;
-
- // See if the statement starts with a comment; if so, move
- // past the comment and get the first token of the actual
- // statement to be executed. Note: must use "startsWith"
- // when looking for the comment delimiters instead of
- // "equals" because there may not be whitespace between the
- // the delimiter and the comment itself, ex "--my comment".
- if (sql.trim().startsWith("--")) {
-
- // Read each line of the statement until we find a
- // line that is NOT a comment.
- int lastEndLine = -1;
- String endline = "\n\r\f";
- tokenizer = new java.util.StringTokenizer(sql, endline, true);
- while (tokenizer.hasMoreTokens()) {
- firstToken = tokenizer.nextToken();
- if (endline.indexOf(firstToken) != -1)
- // this is some sort of newline ("\n", "\r", or "\f").
- lastEndLine = sql.indexOf(firstToken, lastEndLine+1);
- else if (!firstToken.trim().startsWith("--"))
- break;
- }
- if (firstToken.startsWith("--")) {
- // entire statement was just one or more comments; pass it as
- // a query to the server and let the server deal with it.
- sqlMode_ = isQuery__;
- return;
- }
- else {
- // we have a non-comment line; get a tokenizer for the
- // statement beginning at the start of this line.
- tokenizer = new java.util.StringTokenizer(
- sql.substring(lastEndLine+1), delims);
- }
-
- }
- else {
- // there aren't any leading comments, so just get the first token
- // in the SQL statement.
- tokenizer = new java.util.StringTokenizer(sql, delims);
- }
+ sqlUpdateMode_ = 0;
- if (!tokenizer.hasMoreTokens()) {
- throw new SqlException(agent_.logWriter_,
- new ClientMessageId(SQLState.NO_TOKENS_IN_SQL_TEXT), sql);
+ // See if the statement starts with one or more comments; if so, move
+ // past the comments and get the first token of the actual statement to
+ // be executed.
+ String firstToken = getStatementToken(sql);
+
+ if (firstToken == null) {
+ // entire statement was just one or more comments; pass it as a
+ // query to the server and let the server deal with it.
+ sqlMode_ = isQuery__;
+ return;
}
- sqlUpdateMode_ = 0;
- firstToken = tokenizer.nextToken();
if (firstToken.equalsIgnoreCase("select") || // captures <subselect> production
firstToken.equalsIgnoreCase("values")) // captures <values-clause> production
@@ -2385,6 +2347,170 @@
}
}
+
+ /**
+ * Minion of getStatementToken. If the input string starts with an
+ * identifier consisting of letters only (like "select", "update"..),return
+ * it, else return supplied string.
+ * @see #getStatementToken
+ * @param sql input string
+ * @return identifier or unmodified string
+ */
+ private String isolateAnyInitialIdentifier (String sql) {
+ int idx = 0;
+ int length = sql.length();
+
+ if (length == 0) {
+ return sql;
+ }
+
+ char next = sql.charAt(idx);
+
+ if (!Character.isLetter(next)) {
+ return sql;
+ }
+
+ while (idx < length) {
+ if (!Character.isLetter(next)) {
+ break;
+ }
+ next = sql.charAt(++idx);
+ }
+
+ return sql.substring(0, idx);
+ }
+
+ /**
+ * State constants used by the FSM inside getStatementToken.
+ * @see #getStatementToken
+ */
+ private final static int OUTSIDE = 0;
+ private final static int INSIDE_SIMPLECOMMENT = 1;
+ private final static int INSIDE_BRACKETED_COMMENT = 2;
+
+ /**
+ * Step past any initial non-significant characters and comments to find
+ * first significant SQL token so we can classify statement.
+ *
+ * @return first significant SQL token
+ * @throws SqlException std exception policy
+ */
+ private String getStatementToken(String sql) throws SqlException {
+ int bracketNesting = 0;
+ int state = OUTSIDE;
+ int idx = 0;
+ String tokenFound = null;
+ char next;
+
+ while (idx < sql.length() && tokenFound == null) {
+ next = sql.charAt(idx);
+
+ switch (state) {
+ case OUTSIDE:
+ switch (next) {
+ case '\n':
+ case '\t':
+ case '\r':
+ case '\f':
+ case ' ':
+ case '(':
+ case '{': // JDBC escape characters
+ case '=': //
+ case '?': //
+ idx++;
+ break;
+ case '/':
+ if (idx == sql.length() - 1) {
+ // no more characters, so this is the token
+ tokenFound = "/";
+ } else if (sql.charAt(idx + 1) == '*') {
+ state = INSIDE_BRACKETED_COMMENT;
+ bracketNesting++;
+ idx++; // step two chars
+ }
+
+ idx++;
+ break;
+ case '-':
+ if (idx == sql.length() - 1) {
+ // no more characters, so this is the token
+ tokenFound = "/";
+ } else if (sql.charAt(idx + 1) == '-') {
+ state = INSIDE_SIMPLECOMMENT;
+ idx = idx++;
+ }
+
+ idx++;
+ break;
+ default:
+ // a token found
+ tokenFound = isolateAnyInitialIdentifier(
+ sql.substring(idx));
+
+ break;
+ }
+
+ break;
+ case INSIDE_SIMPLECOMMENT:
+ switch (next) {
+ case '\n':
+ case '\r':
+ case '\f':
+
+ state = OUTSIDE;
+ idx++;
+
+ break;
+ default:
+ // anything else inside a simple comment is ignored
+ idx++;
+ break;
+ }
+
+ break;
+ case INSIDE_BRACKETED_COMMENT:
+ switch (next) {
+ case '/':
+ if (idx != sql.length() - 1 &&
+ sql.charAt(idx + 1) == '*') {
+
+ bracketNesting++;
+ idx++; // step two chars
+ }
+ idx++;
+
+ break;
+ case '*':
+ if (idx != sql.length() - 1 &&
+ sql.charAt(idx + 1) == '/') {
+
+ bracketNesting--;
+
+ if (bracketNesting == 0) {
+ state = OUTSIDE;
+ idx++; // step two chars
+ }
+ }
+
+ idx++;
+ break;
+ default:
+ idx++;
+ break;
+ }
+
+ break;
+ default:
+ if (SanityManager.DEBUG) {
+ SanityManager.ASSERT(false);
+ }
+ break;
+ }
+ }
+
+ return tokenFound;
+ }
+
private void parseUpdateSql(String firstToken) throws SqlException {
sqlMode_ = isUpdate__;
if (firstToken.equalsIgnoreCase("insert")) {
Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java?rev=809643&r1=809642&r2=809643&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java Mon Aug 31 16:05:24 2009
@@ -26,6 +26,8 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
+import java.sql.PreparedStatement;
+import java.sql.Types;
import junit.framework.Assert;
import junit.framework.Test;
@@ -54,7 +56,7 @@
*/
public static Test suite()
{
- return TestConfiguration.embeddedSuite(CommentTest.class);
+ return TestConfiguration.defaultSuite(CommentTest.class);
}
/**
@@ -69,6 +71,10 @@
new String [][] {{"1"}});
JDBC.assertFullResultSet(
+ stmt.executeQuery("-- eof comment\nVALUES 1"),
+ new String [][] {{"1"}});
+
+ JDBC.assertFullResultSet(
stmt.executeQuery("VALUES 1 /* a comment */"),
new String [][] {{"1"}});
@@ -129,14 +135,94 @@
// just comments generates syntax error
assertCompileError("42X01", "/* this is a comment */");
+ assertCompileError("42X01", "/* this is a comment */ /* /* foo */ */");
+ assertCompileError(
+ "42X01",
+ "\n\r\r\n/* Weird newlines in front of a comment */" +
+ " /* /* foo */ */");
assertCompileError("42X01", "-- this is a comment \n");
+
+ // sole comment error
+ assertCompileError("42X02", "/* this is not quite a comment");
+ }
+
+
+ /**
+ * Test that an initial bracketed comment doesn't affect the checks for
+ * executeQuery(executeUpdate
+ */
+ public void testInitialComment_derby4338() throws Exception
+ {
+ Statement s = createStatement();
+
+ JDBC.assertDrainResults(
+ s.executeQuery("/* comment */ select * from sys.systables"));
+ JDBC.assertDrainResults(
+ s.executeQuery("/* */\nSELECT * from sys.systables"));
+ JDBC.assertDrainResults(
+ s.executeQuery("/* --*/\n\rSELECT * from sys.systables"));
+ JDBC.assertDrainResults(
+ s.executeQuery("--\nselect * from sys.systables"));
+
+ s.executeUpdate("/* /* foo*/ */ create table t (i int)");
+ s.executeUpdate("--\n drop table t");
+
+ PreparedStatement ps = prepareStatement(
+ "{call syscs_util." +
+ "syscs_set_database_property('foo', ?)}");
+ ps.setString(1, "bar");
+ ps.execute();
+
+ if (usingEmbedded()) {
+ Assert.assertTrue(ps.getUpdateCount() == 0);
+ } else {
+ // Change to 0 when DERBY-211 is fixed.
+ Assert.assertTrue(ps.getUpdateCount() == -1);
+ }
+
+ // The escape after the comment below was not handled correctly prior
+ // to DERBY-4338, i.e. the statement was not classified as a "call"
+ // statement.
+ ps = prepareStatement(
+ "--\n{call syscs_util." +
+ "syscs_set_database_property('foo', ?)}");
+ ps.setString(1, "bar");
+ ps.execute();
+
+ // The assert blows up for the client prior to fix of DERBY-4338.
+ if (usingEmbedded()) {
+ Assert.assertEquals(0, ps.getUpdateCount());
+ } else {
+ // Change to 0 when DERBY-211 is fixed.
+ Assert.assertEquals(-1, ps.getUpdateCount());
+ }
+
+ ps.setNull(1, Types.VARCHAR); // clean up setting
+ ps.execute();
+ }
+
+ /**
+ * Test that an statement classifier in client doesn't get confused over
+ * keywords that end in *, ' and ". This is not strictly a comment test,
+ * but was fixed as part of DERBY-4338.
+ */
+ public void testWrongKeywordLexing_derby4338() throws Exception
+ {
+ Statement s = createStatement();
+
+ JDBC.assertDrainResults(
+ s.executeQuery("select* from sys.systables"));
+ JDBC.assertDrainResults(
+ s.executeQuery("select'a' from sys.systables"));
+ JDBC.assertDrainResults(
+ s.executeQuery("select\"TABLEID\" from sys.systables"));
}
-
+
/**
* Default connections to auto-commit false.
*/
protected void initializeConnection(Connection conn) throws SQLException
- {
+ {
conn.setAutoCommit(false);
}
}