Derby
  1. Derby
  2. DERBY-3573

Argument checking for ResultSet.setFetchSize(int) is incorrect

    Details

    • Urgency:
      Low
    • Issue & fix info:
      Newcomer

      Description

      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)?

      1. derby-3573-test.diff
        1 kB
        Dyre Tjeldvoll
      2. derby-3573-b.diff
        2 kB
        Dyre Tjeldvoll
      3. derby-3573-a.diff
        2 kB
        Nirmal Fernando

        Issue Links

          Activity

          Hide
          Tiago R. Espinha added a comment -

          Triaged for 10.5.2. No changes made.

          Show
          Tiago R. Espinha added a comment - Triaged for 10.5.2. No changes made.
          Hide
          Kathey Marsden added a comment -

          Triage for 10.8. I think it makes sense to change both drivers to comply with latest spec which allows any non-negative value as argument to ResultSet.setFetchSize(int). Does this mean that it will override setMaxRows()?

          Show
          Kathey Marsden added a comment - Triage for 10.8. I think it makes sense to change both drivers to comply with latest spec which allows any non-negative value as argument to ResultSet.setFetchSize(int). Does this mean that it will override setMaxRows()?
          Hide
          Dag H. Wanvik added a comment - - edited

          +1 to making both drivers comply. I think setFetchSize and setMaxRows are orthogonal, but if setFetchSize is set greater than setMaxRows, I think the setting in setMaxRows still apply: the "fetch size" (a "hint") is about how many rows to fetch at a time from the server to the client (trade-off between network round-tips, client memory size, latency for first rows) as far as I understand it. "Max size" is a hard limit for how many rows are visible as part of the result set, no matter whether this is enforced on server or client side.

          Show
          Dag H. Wanvik added a comment - - edited +1 to making both drivers comply. I think setFetchSize and setMaxRows are orthogonal, but if setFetchSize is set greater than setMaxRows, I think the setting in setMaxRows still apply: the "fetch size" (a "hint") is about how many rows to fetch at a time from the server to the client (trade-off between network round-tips, client memory size, latency for first rows) as far as I understand it. "Max size" is a hard limit for how many rows are visible as part of the result set, no matter whether this is enforced on server or client side.
          Hide
          Nirmal Fernando added a comment -

          Attaching a diff!
          Is there a way to find test files related to this change?

          Thanks.

          Show
          Nirmal Fernando added a comment - Attaching a diff! Is there a way to find test files related to this change? Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          StatementJdbc20Test.java has some tests for rs.setFetchSize().

          Show
          Knut Anders Hatlen added a comment - StatementJdbc20Test.java has some tests for rs.setFetchSize().
          Hide
          Dyre Tjeldvoll added a comment -

          Patch a no longer applied cleanly, so attaching an identical patch against the latest trunk. Junit and harness tests passed.

          Show
          Dyre Tjeldvoll added a comment - Patch a no longer applied cleanly, so attaching an identical patch against the latest trunk. Junit and harness tests passed.
          Hide
          Dyre Tjeldvoll added a comment -

          StatementJdbc20Test passed, but only because there was no fail() after setting a fetchSize larger than maxRows. Attaching a test patch to get the new behavior tested.

          Show
          Dyre Tjeldvoll added a comment - StatementJdbc20Test passed, but only because there was no fail() after setting a fetchSize larger than maxRows. Attaching a test patch to get the new behavior tested.
          Hide
          ASF subversion and git services added a comment -

          Commit 1531854 from Dyre Tjeldvoll in branch 'code/trunk'
          [ https://svn.apache.org/r1531854 ]

          DERBY-3573: Remove incorrect check against maxRows when setting fetchSize. Patch contributed by Nirmal Fernando.

          Show
          ASF subversion and git services added a comment - Commit 1531854 from Dyre Tjeldvoll in branch 'code/trunk' [ https://svn.apache.org/r1531854 ] DERBY-3573 : Remove incorrect check against maxRows when setting fetchSize. Patch contributed by Nirmal Fernando.
          Hide
          Mike Matrigali added a comment -

          consider for 10.10 backport

          Show
          Mike Matrigali added a comment - consider for 10.10 backport
          Hide
          Mike Matrigali added a comment -

          Marking not for backport. Change seems like it might affect existing applications, so better to introduce the change to implement the new standard only in a new release.

          Show
          Mike Matrigali added a comment - Marking not for backport. Change seems like it might affect existing applications, so better to introduce the change to implement the new standard only in a new release.

            People

            • Assignee:
              Nirmal Fernando
              Reporter:
              Dyre Tjeldvoll
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development