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/09/01 00:19:32 UTC

svn commit: r809764 - in /db/derby/code/branches/10.4: ./ java/client/org/apache/derby/client/am/Statement.java java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java

Author: dag
Date: Mon Aug 31 22:19:32 2009
New Revision: 809764

URL: http://svn.apache.org/viewvc?rev=809764&view=rev
Log:
DERBY-4338 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.

Backported from trunk as

svn merge -c 809643 https://svn.eu.apache.org/repos/asf/db/derby/code/trunk

Modified:
    db/derby/code/branches/10.4/   (props changed)
    db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Statement.java
    db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java

Propchange: db/derby/code/branches/10.4/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Aug 31 22:19:32 2009
@@ -1 +1 @@
-/db/derby/code/trunk:788436,793588,794303,796316,796372,797147,798347,798742,800523,803548,805696
+/db/derby/code/trunk:788436,793588,794303,796316,796372,797147,798347,798742,800523,803548,805696,809643

Modified: db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Statement.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Statement.java?rev=809764&r1=809763&r2=809764&view=diff
==============================================================================
--- db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Statement.java (original)
+++ db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Statement.java Mon Aug 31 22:19:32 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/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java?rev=809764&r1=809763&r2=809764&view=diff
==============================================================================
--- db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java (original)
+++ db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java Mon Aug 31 22:19:32 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);
     }
 }