Derby
  1. Derby
  2. DERBY-4748

StringIndexOutOfBoundsException on syntax error (invalid COMMIT)

    Details

    • Urgency:
      Normal
    • Bug behavior facts:
      Regression

      Description

      Start the network server on port 1527 on localhost.

      Run ant on the following:

      <project default="all">
      <property environment="env" />
      <path id="derby.classpath.id">
      <fileset dir="${env.DERBY_LIB">
      <include name="derbynet.jar" />
      <include name="derbyclient.jar" />
      </fileset>
      </path>

      <target name="all">
      <sql driver="org.apache.derby.jdbc.ClientDriver"

      url="jdbc:derby://localhost:1527/derbyDB;create=true;user=derbyuser;passsword=derbypwd"
      userid="derbyuser"
      password="derbypwd"
      classpathref="derby.classpath.id"
      onerror="continue">
      create table mytable1 (mycol varchar(255));
      commit;
      </sql>
      </target>
      </project>

      On 10.5.3, I get
      [sql] Executing commands
      [sql] Failed to execute: commit
      [sql] java.sql.SQLSyntaxErrorException: Syntax error: Encountered
      "commit"
      at line 1, column 1.
      [sql] 1 of 2 SQL statements executed successfully

      but on 10.6.1, I get
      [sql] Executing commands

      BUILD FAILED
      java.lang.StringIndexOutOfBoundsException: String index out of range: 6
      at java.lang.String.charAt(String.java:686)
      at org.apache.derby.client.am.Statement.isolateAnyInitialIdentifier(Unknown Source)
      at org.apache.derby.client.am.Statement.getStatementToken(Unknown Source)
      at org.apache.derby.client.am.Statement.parseSqlAndSetSqlModes(Unknown Source)
      ....

      1. derby-4748-1b-sioobe.diff
        3 kB
        Kristian Waagan
      2. derby-4748-1a-sioobe.diff
        2 kB
        Kristian Waagan

        Activity

        Stephen Felts created issue -
        Hide
        Kristian Waagan added a comment -

        Thanks for the report.

        This bug was introduced under DERBY-4338. I'll have a look.

        Show
        Kristian Waagan added a comment - Thanks for the report. This bug was introduced under DERBY-4338 . I'll have a look.
        Kristian Waagan made changes -
        Field Original Value New Value
        Assignee Kristian Waagan [ kristwaa ]
        Affects Version/s 10.4.2.1 [ 12313401 ]
        Affects Version/s 10.5.3.1 [ 12314182 ]
        Affects Version/s 10.7.0.0 [ 12314971 ]
        Component/s Network Client [ 11690 ]
        Hide
        Kristian Waagan added a comment -

        Attaching patch 1a, which changes the logic in the loop.
        I think there are many ways to write this logic/loop, and almost as many ways to get it wrong

        Regression tests passed.
        Patch ready for review.

        Show
        Kristian Waagan added a comment - Attaching patch 1a, which changes the logic in the loop. I think there are many ways to write this logic/loop, and almost as many ways to get it wrong Regression tests passed. Patch ready for review.
        Kristian Waagan made changes -
        Attachment derby-4748-1a-sioobe.diff [ 12449917 ]
        Kristian Waagan made changes -
        Issue & fix info [Patch Available]
        Hide
        Knut Anders Hatlen added a comment -

        The fix looks correct to me. +1 to commit.

        Minor nit: The helper method assertCompileError() could be used to simplify the test case.

        Show
        Knut Anders Hatlen added a comment - The fix looks correct to me. +1 to commit. Minor nit: The helper method assertCompileError() could be used to simplify the test case.
        Hide
        Knut Anders Hatlen added a comment -

        By the way, I think the isolateAnyInitialIdentifier() method is somewhat more complex than it needs to be in the first place. Replacing it with something like the (untested) loop below would probably solve the out-of-bounds exception and also make the method easier to read since it has fewer special cases to check for.

        int idx;
        for (idx = 0; idx < sql.length(); idx++) {
        char ch = sql.charAt(idx);
        if (!Character.isLetter(ch))

        { // first non-token char found break; }

        }
        // return initial token if one is found, or the entire string otherwise
        return (idx > 0) ? sql.substring(0, idx) : sql;

        Show
        Knut Anders Hatlen added a comment - By the way, I think the isolateAnyInitialIdentifier() method is somewhat more complex than it needs to be in the first place. Replacing it with something like the (untested) loop below would probably solve the out-of-bounds exception and also make the method easier to read since it has fewer special cases to check for. int idx; for (idx = 0; idx < sql.length(); idx++) { char ch = sql.charAt(idx); if (!Character.isLetter(ch)) { // first non-token char found break; } } // return initial token if one is found, or the entire string otherwise return (idx > 0) ? sql.substring(0, idx) : sql;
        Hide
        Kristian Waagan added a comment -

        Thanks, Knut.
        That code looks better.

        I uploaded a new revision of the patch (1b), which uses your code and also includes some more changes to the test.
        Regression tests passed, I expect to commit it shortly.

        Show
        Kristian Waagan added a comment - Thanks, Knut. That code looks better. I uploaded a new revision of the patch (1b), which uses your code and also includes some more changes to the test. Regression tests passed, I expect to commit it shortly.
        Kristian Waagan made changes -
        Attachment derby-4748-1b-sioobe.diff [ 12450149 ]
        Hide
        Knut Anders Hatlen added a comment -

        +1

        Show
        Knut Anders Hatlen added a comment - +1
        Hide
        Kristian Waagan added a comment -

        Committed patch 1b to trunk with revision 980684.
        Will backport before closing.

        Show
        Kristian Waagan added a comment - Committed patch 1b to trunk with revision 980684. Will backport before closing.
        Kristian Waagan made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Issue & fix info [Patch Available]
        Fix Version/s 10.7.0.0 [ 12314971 ]
        Resolution Fixed [ 1 ]
        Hide
        Kristian Waagan added a comment -

        Now I understand why I'm not supposed to resolve the issue before I have back-ported the fix

        Show
        Kristian Waagan added a comment - Now I understand why I'm not supposed to resolve the issue before I have back-ported the fix
        Kristian Waagan made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Kristian Waagan added a comment -

        Backported to 10.6 with revision 984074, 10.5 with revision 984079, and finally to 10.4 with revision 984080. suites.All and derbyall ran without failures, with the exception of store/aes.sql on 10.4.

        Stephen, the issue is ready for verification and closing.

        Show
        Kristian Waagan added a comment - Backported to 10.6 with revision 984074, 10.5 with revision 984079, and finally to 10.4 with revision 984080. suites.All and derbyall ran without failures, with the exception of store/aes.sql on 10.4. Stephen, the issue is ready for verification and closing.
        Kristian Waagan made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Fix Version/s 10.4.2.1 [ 12313401 ]
        Fix Version/s 10.5.3.1 [ 12314182 ]
        Fix Version/s 10.6.1.1 [ 12314973 ]
        Resolution Fixed [ 1 ]
        Kathey Marsden made changes -
        Fix Version/s 10.6.2.0 [ 12315342 ]
        Fix Version/s 10.6.1.1 [ 12314973 ]
        Knut Anders Hatlen made changes -
        Fix Version/s 10.6.2.0 [ 12315342 ]
        Fix Version/s 10.6.2.1 [ 12315343 ]
        Rick Hillegas made changes -
        Affects Version/s 10.7.1.1 [ 12315564 ]
        Affects Version/s 10.7.1.0 [ 12314971 ]
        Fix Version/s 10.7.1.1 [ 12315564 ]
        Fix Version/s 10.7.1.0 [ 12314971 ]
        Hide
        Knut Anders Hatlen added a comment -

        [bulk update] Close all resolved issues that haven't been updated for more than one year.

        Show
        Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.
        Knut Anders Hatlen made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Gavin made changes -
        Workflow jira [ 12516076 ] Default workflow, editable Closed status [ 12802800 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        10d 9h 43m 1 Kristian Waagan 30/Jul/10 09:56
        Resolved Resolved Reopened Reopened
        11d 6h 18m 1 Kristian Waagan 10/Aug/10 16:15
        Reopened Reopened Resolved Resolved
        4m 47s 1 Kristian Waagan 10/Aug/10 16:20
        Resolved Resolved Closed Closed
        1041d 17h 59m 1 Knut Anders Hatlen 17/Jun/13 10:19

          People

          • Assignee:
            Kristian Waagan
            Reporter:
            Stephen Felts
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development