Derby
  1. Derby
  2. DERBY-5533

Client differs from embedded when rs.updateInt overflows: 22015 vs 22003

    Details

    • Bug behavior facts:
      Deviation from standard, Embedded/Client difference

      Description

      stm.executeUpdate("create table t(i smallint)");
      stm.executeUpdate("insert into t values 1,2,3,4");

      ResultSet rs = stm.executeQuery("select i from t");

      rs.next();

      try

      { rs.updateInt(1, 100000); }

      catch (SQLException e)

      { // client: 22015 vs embedded 22003 }

      According to the standard, 22015 should be used for INTERVALs ("interval field overflow"). 22003 seems more correct, the standard uses that for "numeric value out of range".

      1. derby-5533b.stat
        1 kB
        Dag H. Wanvik
      2. derby-5533b.diff
        63 kB
        Dag H. Wanvik
      3. derby-5533.stat
        1 kB
        Dag H. Wanvik
      4. derby-5533.diff
        56 kB
        Dag H. Wanvik
      5. derby-5533-test.diff
        14 kB
        Dag H. Wanvik
      6. derby-5533-repro.diff
        6 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Kathey Marsden added a comment -

          Label derby_backport_reject_10_8. Shouldn't backport SQLState change

          Show
          Kathey Marsden added a comment - Label derby_backport_reject_10_8. Shouldn't backport SQLState change
          Hide
          Dag H. Wanvik added a comment -

          Committed version "b" of the patch as svn 1215365, resolving.

          Show
          Dag H. Wanvik added a comment - Committed version "b" of the patch as svn 1215365, resolving.
          Hide
          Dag H. Wanvik added a comment -

          Uploading a new version of the patch, "derby-5513b". It adds more tests for the floating point types. This led to the posting of DERBY-5546, too. The test has many test cases that should be fixed up when the follow-on issues (all linked to this issue) are fixed.

          Show
          Dag H. Wanvik added a comment - Uploading a new version of the patch, "derby-5513b". It adds more tests for the floating point types. This led to the posting of DERBY-5546 , too. The test has many test cases that should be fixed up when the follow-on issues (all linked to this issue) are fixed.
          Hide
          Dag H. Wanvik added a comment -

          Thanks, Knut! Regressions passed, modulo DERBY-1913.

          Show
          Dag H. Wanvik added a comment - Thanks, Knut! Regressions passed, modulo DERBY-1913 .
          Hide
          Knut Anders Hatlen added a comment -

          The patch looks good to me. +1

          Show
          Knut Anders Hatlen added a comment - The patch looks good to me. +1
          Hide
          Dag H. Wanvik added a comment -

          Thanks, Rick. I decided to move them to ParameterMappingTest instead; only half the new tests concern updatable result sets, after all.

          Uploading patch derby-5533 with the tests and a fix which changes the class LossOfPrecisionConversionException to OutsideRangeForDataTypeException, wrapping 22003 and updates the usage sites, all of which are covered by the tests I believe. I also removed the old error 22015.S.1, which was only used by the client.

          Note: whereas the old error message indicated the value that was out of range for the conversion, the 22003 erro rmessage shows the target type XXX for setXXX. However, for getXXX, the type given in the error message by the embedded driver not the target Java type (e.g. short in getShort), but the correspondig SQL type, i,e.. SMALLINT. I think this is the way it is because on the server, the same conversion methods are also used in CAST, where the target type is an SQL type, not a Java type. I chose to let the client use the same type strings as embedded does. If we want to improve on this later, we can do that for both drivers.

          Running regressions, please review.

          Show
          Dag H. Wanvik added a comment - Thanks, Rick. I decided to move them to ParameterMappingTest instead; only half the new tests concern updatable result sets, after all. Uploading patch derby-5533 with the tests and a fix which changes the class LossOfPrecisionConversionException to OutsideRangeForDataTypeException, wrapping 22003 and updates the usage sites, all of which are covered by the tests I believe. I also removed the old error 22015.S.1, which was only used by the client. Note: whereas the old error message indicated the value that was out of range for the conversion, the 22003 erro rmessage shows the target type XXX for setXXX. However, for getXXX, the type given in the error message by the embedded driver not the target Java type (e.g. short in getShort), but the correspondig SQL type, i,e.. SMALLINT. I think this is the way it is because on the server, the same conversion methods are also used in CAST, where the target type is an SQL type, not a Java type. I chose to let the client use the same type strings as embedded does. If we want to improve on this later, we can do that for both drivers. Running regressions, please review.
          Hide
          Rick Hillegas added a comment -

          Thanks for the impressive battery of new tests, Dag! ParameterMappingTest doesn't have a header comment explaining its purpose in life, but it seems to try to run a comprehensive battery of tests against the getXXX() and setXXX() methods. UpdateXXXTest, according to its header comment, is a test for updatable ResultSets. If this fix is not specific to updatable ResultSets, ParameterMappingTest might be a better home for it. But I don't think it would hurt to leave the tests where you put them originally. Thanks!

          Show
          Rick Hillegas added a comment - Thanks for the impressive battery of new tests, Dag! ParameterMappingTest doesn't have a header comment explaining its purpose in life, but it seems to try to run a comprehensive battery of tests against the getXXX() and setXXX() methods. UpdateXXXTest, according to its header comment, is a test for updatable ResultSets. If this fix is not specific to updatable ResultSets, ParameterMappingTest might be a better home for it. But I don't think it would hurt to leave the tests where you put them originally. Thanks!
          Hide
          Dag H. Wanvik added a comment -

          Uploading derby-5522-test which extends the repro into a full fledged test of upper lower bound checking on the conversions in ResultSet#setXXX and #getXXX.

          I have checked that all usages of LossOfPrecisionConversionException is covered by these tests, so changing that exception should be safe.

          Up till now I have made this a patch of UpdateXXXTest, which may not be entirely suitable now that we're testing getXXX as well. Poked around, but found only ParameterMappingTest asserting against the SQL state 22003. Should I try to move these new tests over there, possibly make a new test, or just leave them inside UpdateXXXTest?

          Show
          Dag H. Wanvik added a comment - Uploading derby-5522-test which extends the repro into a full fledged test of upper lower bound checking on the conversions in ResultSet#setXXX and #getXXX. I have checked that all usages of LossOfPrecisionConversionException is covered by these tests, so changing that exception should be safe. Up till now I have made this a patch of UpdateXXXTest, which may not be entirely suitable now that we're testing getXXX as well. Poked around, but found only ParameterMappingTest asserting against the SQL state 22003. Should I try to move these new tests over there, possibly make a new test, or just leave them inside UpdateXXXTest?
          Hide
          Dag H. Wanvik added a comment -

          Data point: changing the SQL state in LossOfPrecisionConversionException to 22003 makes the repro test pass.

          Show
          Dag H. Wanvik added a comment - Data point: changing the SQL state in LossOfPrecisionConversionException to 22003 makes the repro test pass.
          Hide
          Dag H. Wanvik added a comment -

          Uploading derby-5533-repro, which reproduces this in the form of a patch against jdbcapi.UpdateXXXTest. It tests all upper bounds checks needed for updateXXX in a new fixture.
          In addition, lower bound checks should be added to it.

          LossOfPrecisionConversionException is also thrown from the methods CrossConverter#getXXXFromYYY, so that usage should be checked as well (probably also wrong).

          Show
          Dag H. Wanvik added a comment - Uploading derby-5533-repro, which reproduces this in the form of a patch against jdbcapi.UpdateXXXTest. It tests all upper bounds checks needed for updateXXX in a new fixture. In addition, lower bound checks should be added to it. LossOfPrecisionConversionException is also thrown from the methods CrossConverter#getXXXFromYYY, so that usage should be checked as well (probably also wrong).
          Hide
          Dag H. Wanvik added a comment -

          The code generating 22015 in the client is LossOfPrecisionConversionException. I noticed this was unused in the regressions suite coverage, so presumably out JUnit
          tests do not test this error condition, at least not on the client.

          Show
          Dag H. Wanvik added a comment - The code generating 22015 in the client is LossOfPrecisionConversionException. I noticed this was unused in the regressions suite coverage, so presumably out JUnit tests do not test this error condition, at least not on the client.

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development