Derby
  1. Derby
  2. DERBY-3203

Convert jdbcapi/statementJdbc20.java to JUnit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.4.1.3
    • Component/s: Test
    • Labels:
      None

      Description

      This issue is creates for converting jdbcapi/statementJdbc20.java to JUnit.

      1. DERBY-3203v3.diff
        4 kB
        Dyre Tjeldvoll
      2. DERBY-3203v2.diff
        20 kB
        Ramin Moazeni
      3. DERBY-3203v0.stat
        0.6 kB
        Ramin Moazeni
      4. DERBY-3203v0.diff
        20 kB
        Ramin Moazeni

        Issue Links

          Activity

          Hide
          Ramin Moazeni added a comment -

          Hello

          I am attaching the patch to this issue.
          I appreciate your review/comments.

          Thanks
          Ramin

          Show
          Ramin Moazeni added a comment - Hello I am attaching the patch to this issue. I appreciate your review/comments. Thanks Ramin
          Hide
          Kathey Marsden added a comment -

          Hi Ramin, I realize this is probably just carry over from the previous test, but I wonder if it makes more sense to use ResultSet.FETCH_REVERSE instead of 1001.

          assertEquals(stmt.getFetchDirection(), 1001);

          Show
          Kathey Marsden added a comment - Hi Ramin, I realize this is probably just carry over from the previous test, but I wonder if it makes more sense to use ResultSet.FETCH_REVERSE instead of 1001. assertEquals(stmt.getFetchDirection(), 1001);
          Hide
          Ramin Moazeni added a comment -

          Hi Kathey

          Thanks for the comment. I am attaching a new
          patch with this issue.

          Thanks
          Ramin

          Show
          Ramin Moazeni added a comment - Hi Kathey Thanks for the comment. I am attaching a new patch with this issue. Thanks Ramin
          Hide
          Dyre Tjeldvoll added a comment -

          Committed revision 601058.

          Show
          Dyre Tjeldvoll added a comment - Committed revision 601058.
          Hide
          Daniel John Debrunner added a comment -

          Test has several try/catch blocks where I think a failure is expected but the test will pass if the try portion succeeds as there is no fail() assert.
          E.g.

          try

          { rs.setFetchDirection(ResultSet.FETCH_FORWARD); }

          catch(SQLException e)

          { if (usingEmbedded()) assertSQLState("XJ061", e); else assertSQLState("XJ125", e); }
          Show
          Daniel John Debrunner added a comment - Test has several try/catch blocks where I think a failure is expected but the test will pass if the try portion succeeds as there is no fail() assert. E.g. try { rs.setFetchDirection(ResultSet.FETCH_FORWARD); } catch(SQLException e) { if (usingEmbedded()) assertSQLState("XJ061", e); else assertSQLState("XJ125", e); }
          Hide
          Dyre Tjeldvoll added a comment -

          Hi Ramin,

          I think Dan is making a very good point about the missing fail()
          asserts in some of the try-blocks.

          I've attached a patch (v3) which adds the missing fail()-asserts.
          Unfortunately, it also reveals that one of the test
          cases is not throwing the expected exception

          The statement that doesn't fail is 'rs.setFetchSize(100)' in
          'testLocalValuesOfResultSet()' in client mode. In v3 I have
          enclosed the fail()-assert in an usingEmbedded()-test just to
          make the test pass, but I don't know if this is expected
          behavior or a bug. What do you think?

          Show
          Dyre Tjeldvoll added a comment - Hi Ramin, I think Dan is making a very good point about the missing fail() asserts in some of the try-blocks. I've attached a patch (v3) which adds the missing fail()-asserts. Unfortunately, it also reveals that one of the test cases is not throwing the expected exception The statement that doesn't fail is 'rs.setFetchSize(100)' in 'testLocalValuesOfResultSet()' in client mode. In v3 I have enclosed the fail()-assert in an usingEmbedded()-test just to make the test pass, but I don't know if this is expected behavior or a bug. What do you think?
          Hide
          Dyre Tjeldvoll added a comment -

          I briefly checked what the javadoc and the JDBC tutorial says about ResultSet.setFetchSize(), and I cannot see anything which suggests that rs.setFetchSize(100) is illegal in this context, and the error message "XJ062.S=Invalid parameter value ''

          {0}

          '' for ResultSet.setFetchSize(int rows)." doesn't provide any clues either. So does anyone know why this fails in embedded mode?

          Show
          Dyre Tjeldvoll added a comment - I briefly checked what the javadoc and the JDBC tutorial says about ResultSet.setFetchSize(), and I cannot see anything which suggests that rs.setFetchSize(100) is illegal in this context, and the error message "XJ062.S=Invalid parameter value '' {0} '' for ResultSet.setFetchSize(int rows)." doesn't provide any clues either. So does anyone know why this fails in embedded mode?
          Hide
          Dyre Tjeldvoll added a comment -

          I think I have resolved the apparent mystery here. The requirement that the argument to ResultSet.setFetchSize(int) be less than Statement.getMaxRows() was dropped in Java 6/JDBC 4, (it is not present in the Java 6 javadoc, but can still be seen in the Java 5 javadoc).

          The reason why the client driver doesn't throw an exception in this case is because am.ResultSet incorrectly checks against ResultSet.maxRows_ and NOT am.Statement.getMaxRows(). So when am.Statement.setMaxRows(int) is called after a result set has already been created, am.ResultSet.setFechSize(int) will check against a stale value.

          The question is what to do about this. The client driver clearly has a bug, but should we fix it by duplicating the old behavior found in the embedded driver, or change both drivers to comply with latest spec which allows any non-negative value as argument to ResultSet.setFetchSize(int)?

          Show
          Dyre Tjeldvoll added a comment - I think I have resolved the apparent mystery here. The requirement that the argument to ResultSet.setFetchSize(int) be less than Statement.getMaxRows() was dropped in Java 6/JDBC 4, (it is not present in the Java 6 javadoc, but can still be seen in the Java 5 javadoc). The reason why the client driver doesn't throw an exception in this case is because am.ResultSet incorrectly checks against ResultSet.maxRows_ and NOT am.Statement.getMaxRows(). So when am.Statement.setMaxRows(int) is called after a result set has already been created, am.ResultSet.setFechSize(int) will check against a stale value. The question is what to do about this. The client driver clearly has a bug, but should we fix it by duplicating the old behavior found in the embedded driver, or change both drivers to comply with latest spec which allows any non-negative value as argument to ResultSet.setFetchSize(int)?
          Hide
          Bryan Pendleton added a comment -

          I believe that the 4-Dec commit (revision 601058) had a small mistake
          in it; I believe that the intent of the patch was to delete statementJdbc20.java
          and statementJdbc20.out, as shown by DERBY-3203v0.stat. However, the
          actual commit left those files present and empty (0 bytes).

          I propose to run
          svn delete java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/statementJdbc20.java
          svn delete java/testing/org/apache/derbyTesting/functionTests/master/statementJdbc20.out
          and commit those deletions to the trunk.

          Please let me know if I'm misunderstanding the issue.

          Show
          Bryan Pendleton added a comment - I believe that the 4-Dec commit (revision 601058) had a small mistake in it; I believe that the intent of the patch was to delete statementJdbc20.java and statementJdbc20.out, as shown by DERBY-3203 v0.stat. However, the actual commit left those files present and empty (0 bytes). I propose to run svn delete java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/statementJdbc20.java svn delete java/testing/org/apache/derbyTesting/functionTests/master/statementJdbc20.out and commit those deletions to the trunk. Please let me know if I'm misunderstanding the issue.
          Hide
          Bryan Pendleton added a comment -

          Committed the two deletes to the trunk as revision 616834.

          Show
          Bryan Pendleton added a comment - Committed the two deletes to the trunk as revision 616834.
          Hide
          Dyre Tjeldvoll added a comment -

          Sorry Bryan, I'm a bit behind on my Jira mail. Yes I think you are right. Thanks for fixing this.

          Show
          Dyre Tjeldvoll added a comment - Sorry Bryan, I'm a bit behind on my Jira mail. Yes I think you are right. Thanks for fixing this.
          Hide
          Dyre Tjeldvoll added a comment -

          Can this issue be resloved/closed now?

          Show
          Dyre Tjeldvoll added a comment - Can this issue be resloved/closed now?
          Hide
          Dyre Tjeldvoll added a comment -

          Resolving since the problem with ResultSet.setFetchSize() has been factored out in a separate issue (DERBY-3573)

          Show
          Dyre Tjeldvoll added a comment - Resolving since the problem with ResultSet.setFetchSize() has been factored out in a separate issue ( DERBY-3573 )

            People

            • Assignee:
              Ramin Moazeni
              Reporter:
              Ramin Moazeni
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development