db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d..@apache.org
Subject svn commit: r809763 - in /db/derby/code/branches/10.5: ./ java/client/org/apache/derby/client/am/Statement.java java/testing/org/apache/derbyTesting/functionTests/tests/lang/CommentTest.java
Date Mon, 31 Aug 2009 22:15:27 GMT
Author: dag
Date: Mon Aug 31 22:15:27 2009
New Revision: 809763

URL: http://svn.apache.org/viewvc?rev=809763&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.

Back-ported from trunk as:

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

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

Propchange: db/derby/code/branches/10.5/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Aug 31 22:15:27 2009
@@ -1 +1 @@
-/db/derby/code/trunk:769596,769602,769606,769962,772090,772337,772449,772534,774281,777105,779681,782991,785131,785139,785163,785570,785662,788369,788670,788674,788968,789264,790218,792434,793089,793588,794106,794303,794955,795166,796020,796027,796316,796372,797147,798347,798742,800523,803548,803948,805696,808494,808850
+/db/derby/code/trunk:769596,769602,769606,769962,772090,772337,772449,772534,774281,777105,779681,782991,785131,785139,785163,785570,785662,788369,788670,788674,788968,789264,790218,792434,793089,793588,794106,794303,794955,795166,796020,796027,796316,796372,797147,798347,798742,800523,803548,803948,805696,808494,808850,809643

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



Mime
View raw message