Derby
  1. Derby
  2. DERBY-1252

Old clients with new server return wrong database metadata values for some methods

    Details

    • Urgency:
      Urgent
    • Bug behavior facts:
      Regression

      Description

      With an old client (10.1.1, 10.1.2) accessing a new (10.2) server,
      some metadata calls will return the wrong value for both the JCC and
      the Derby clients:

      deletesAreDetected(TYPE_SCROLL_INSENSITIVE) -> true
      updatedAreDetected(TYPE_SCROLL_INSENSITIVE) -> true
      ownDeletesAreVisible(TYPE_SCROLL_INSENSITIVE) -> true
      ownUpdatesAreVisible(TYPE_SCROLL_INSENSITIVE) -> true

      This happens because these values were changed for the 10.2 with
      the addition of updatable scrollable insensitive result sets (DERBY-775),
      combined with a weakness in the way the client and the server
      cooperates to answer these metadata calls.

      Presently, when the client application invokes these methods, the
      results will be returned by the server without regard to the identity
      of the client, i.e. the 2-tuple

      {JCC or Derby client, client version}

      .
      The values to be returned for the methods in question are based solely
      on the values found in the file metadata_net.properties, which is part
      of the server.

      In general, some database metadata is dependent on the combination of
      the capabilities in the client and the server and the returned values
      should reflect this, which in general implies negotiating (down) to
      values which are correct for the actual combination of client and
      server.

      1. derby1252.diff
        9 kB
        Dag H. Wanvik
      2. derby1252-10.1.diff
        27 kB
        Dag H. Wanvik
      3. derby1252-10.1.stat
        0.6 kB
        Dag H. Wanvik
      4. derby1252.diff
        8 kB
        Dag H. Wanvik
      5. derby1252.stat
        0.4 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Kathey Marsden added a comment -

          This issue is a regression. It would be ideal to release Derby 10.2 without any known regressions so we don't have to even think about where to draw the line for acceptable regressions, so before I think too much about where to draw that line, I ask:

          • How hard this is to fix?
          • Does anyone plan to pick this up for 10.2?
          Show
          Kathey Marsden added a comment - This issue is a regression. It would be ideal to release Derby 10.2 without any known regressions so we don't have to even think about where to draw the line for acceptable regressions, so before I think too much about where to draw that line, I ask: How hard this is to fix? Does anyone plan to pick this up for 10.2?
          Hide
          Daniel John Debrunner added a comment -

          How would this regression manifest itself in a working application?

          Show
          Daniel John Debrunner added a comment - How would this regression manifest itself in a working application?
          Hide
          Dag H. Wanvik added a comment -

          Impact:

          This regression affects client which do not support updatable,
          scrollable, insensitive result sets; i.e. JCC and Derby client driver
          <= 10.1.x.

          Since the metadata methods which experience the regression all concern
          either detectability or visibility of updatable, scrollable,
          insensitive result sets, which is a new feature in 10.2, it seems a
          long stretch to imagine existing applications being negatively
          impacted: Why would an application ask about details of a feature not
          supported? [DataBaseMetaData#supportsResultSetConcurrency and
          supportsResultSetType should be used to determine if SUR is supported,
          and these work correctly for the mixed scenarios (derby 10.1/derby
          10.2 server and jcc client/derby 10.2 server¹).

          ¹Albeit due to luck; a bug in the parsing of a returned metadata
          column in both JCC and Derby client <= 10.1, see also this thread:
          http://www.mail-archive.com/derby-dev@db.apache.org/msg18334.html)

          In general, though, the bug reveals an exisiting weakness in the
          design of the mechanism to answer database metadata methods on the
          client; the server does not take the client version into account when
          answering (negotiate down). Mostly, this should only be a problem when
          the server is newer (has more features) than the client can support.

          Possible Solutions:

          One solution could be: If in a mixed scenario, make the stored
          procedure SYSIBM.METADATA() being executed on the server on behalf of
          the client, return data from a 10.1-compatible version of the
          metadata.

          The query METADATA in the file metadata_net.properties is used to construct
          the stored prepared statement used by SYSIBM.METADATA(). We could have
          a "client <= 10.1" version of the query which can be referenced by
          SYSIBM.METADATA(). This presupposes that the JCC driver also accesses
          the metadata via the stored procedure SYSIBM.METADATA().

          Downside: Redundancy in data, possible proliferation of more version
          specific versions later? I presume we need to keep them
          around til we hit Derby 11.

          An alternative (more ad hoc and ugly) would be to always let the
          server return false for these metadata calls, and let the derby client
          ignore the values returned and hardcode the values for these methods
          into the derby driver based on which version of the server it is
          talking to: server < 10.2 false, server >= 10.2 : true.

          Not sure I like either of the approaches.. But is the regression too
          risky to accept?

          Show
          Dag H. Wanvik added a comment - Impact: This regression affects client which do not support updatable, scrollable, insensitive result sets; i.e. JCC and Derby client driver <= 10.1.x. Since the metadata methods which experience the regression all concern either detectability or visibility of updatable, scrollable, insensitive result sets, which is a new feature in 10.2, it seems a long stretch to imagine existing applications being negatively impacted: Why would an application ask about details of a feature not supported? [DataBaseMetaData#supportsResultSetConcurrency and supportsResultSetType should be used to determine if SUR is supported, and these work correctly for the mixed scenarios (derby 10.1/derby 10.2 server and jcc client/derby 10.2 server¹). ¹Albeit due to luck; a bug in the parsing of a returned metadata column in both JCC and Derby client <= 10.1, see also this thread: http://www.mail-archive.com/derby-dev@db.apache.org/msg18334.html ) In general, though, the bug reveals an exisiting weakness in the design of the mechanism to answer database metadata methods on the client; the server does not take the client version into account when answering (negotiate down). Mostly, this should only be a problem when the server is newer (has more features) than the client can support. Possible Solutions: One solution could be: If in a mixed scenario, make the stored procedure SYSIBM.METADATA() being executed on the server on behalf of the client, return data from a 10.1-compatible version of the metadata. The query METADATA in the file metadata_net.properties is used to construct the stored prepared statement used by SYSIBM.METADATA(). We could have a "client <= 10.1" version of the query which can be referenced by SYSIBM.METADATA(). This presupposes that the JCC driver also accesses the metadata via the stored procedure SYSIBM.METADATA(). Downside: Redundancy in data, possible proliferation of more version specific versions later? I presume we need to keep them around til we hit Derby 11. An alternative (more ad hoc and ugly) would be to always let the server return false for these metadata calls, and let the derby client ignore the values returned and hardcode the values for these methods into the derby driver based on which version of the server it is talking to: server < 10.2 false, server >= 10.2 : true. Not sure I like either of the approaches.. But is the regression too risky to accept?
          Hide
          Rick Hillegas added a comment -

          It seems to me that SYSIBM.METADATA exists to support the JCC driver. Maybe we could let this procedure continue to return the 10.1 answers; the 10.2 client could return the 10.2 answers when talking to a 10.2 server.

          Show
          Rick Hillegas added a comment - It seems to me that SYSIBM.METADATA exists to support the JCC driver. Maybe we could let this procedure continue to return the 10.1 answers; the 10.2 client could return the 10.2 answers when talking to a 10.2 server.
          Hide
          Dag H. Wanvik added a comment -

          Yes, that's what I had in mind for my "alternative two" above. If
          people are OK with that approach I can make a patch for it.

          People finding this solution unacceptable should speak out now. I will
          start on it in within a couple of days if there is no push back.

          Show
          Dag H. Wanvik added a comment - Yes, that's what I had in mind for my "alternative two" above. If people are OK with that approach I can make a patch for it. People finding this solution unacceptable should speak out now. I will start on it in within a couple of days if there is no push back.
          Hide
          Dag H. Wanvik added a comment -

          Two patches are uploaded to solve this issue.

          derby1252-10.1.

          {diff,stat}
          derby1252.{diff,stat}

          The first patch to the 10.1 trunk contains extensions to
          jdbcapi/metadata_test.java (plus updated masters, hope I got them
          all..) which can be used to verify the regression; the old version did
          not contain the calls to the methods of interest, see below. I am not
          sure it is necessary to commit this patch, but it is useful when
          verifying the main patch.

          The main patch to 10.2. trunk implements "alternative 2" outlined
          earlier in this issue. BTW, alternative 1 is not workable since all
          logic on the server side is generic query processing making it ugly to
          differentiate between clients.

          Patch details:

          • metadata_net.properties has been rolled back to the 10.1 version of
            the four values which gave regression:

          deletesAreDetected(TYPE_SCROLL_INSENSITIVE)
          updatedAreDetected(TYPE_SCROLL_INSENSITIVE)
          ownDeletesAreVisible(TYPE_SCROLL_INSENSITIVE)
          ownUpdatesAreVisible(TYPE_SCROLL_INSENSITIVE)

          Derbynet master has been updated to reflect this (existing master
          reflects the regression).

          • am/DatabaseMetaData.java contains logic to override these to "true"
            when running against a >= 10.2. server. I also added code, which is
            presently commented out, to negotiate down (effectivly an "AND" for
            these boolean values) once the server can start returning the
            correct values. As far as I can understand, this cannot happen until
            we hit the next major release, 11, due to the forward compatibility
            requirements for client/server,
            cf. http://wiki.apache.org/db-derby/ForwardCompatibility.
          • If the present solution is committed I will file a JIRA to be fixed
            for Derby 11 (not possible yet?) , to move to the new regime from
            Derby 11.0.

          Testing:

          • Ran derbyall on 10.1 trunk with the extended 10.1 metadata test
            (derby1252-10.1.diff):

          derbyall/derbynetmats/derbynetmats.fail:derbynet/DerbyNetAutoStart.java
          derbyall/derbyall.fail:unit/T_Diagnosticable.unit

          This is unrelated.

          • Ran derbyall on 10.2 trunk with the patch (derby1252.diff):
            wisconsin fails for all clients, plus store/TransactionTable.sql
            (same as tinderbox).
          • Ran 10.1 client and extended 10.1 metadata test against current 10.2
            to show regression: Ok, shows regression.
          • Ran JCC client and extended 10.1 metadata test against current 10.2
            to show regression: Ok, shows regression.
          • Ran 10.1 client and extended 10.1 metadata test against patched 10.2
            to show regression has gone away: OK.
          • Ran JCC client and extended 10.1 metadata test against patched 10.2
            to show regression has gone away: OK.
          • Ran patched 10.2 client against 10.1 server and and extended 10.1
            metadata test to see that the patched 10.2 client downgrades
            correctly: OK

          The patch is ready for review.

          Show
          Dag H. Wanvik added a comment - Two patches are uploaded to solve this issue. derby1252-10.1. {diff,stat} derby1252.{diff,stat} The first patch to the 10.1 trunk contains extensions to jdbcapi/metadata_test.java (plus updated masters, hope I got them all..) which can be used to verify the regression; the old version did not contain the calls to the methods of interest, see below. I am not sure it is necessary to commit this patch, but it is useful when verifying the main patch. The main patch to 10.2. trunk implements "alternative 2" outlined earlier in this issue. BTW, alternative 1 is not workable since all logic on the server side is generic query processing making it ugly to differentiate between clients. Patch details: metadata_net.properties has been rolled back to the 10.1 version of the four values which gave regression: deletesAreDetected(TYPE_SCROLL_INSENSITIVE) updatedAreDetected(TYPE_SCROLL_INSENSITIVE) ownDeletesAreVisible(TYPE_SCROLL_INSENSITIVE) ownUpdatesAreVisible(TYPE_SCROLL_INSENSITIVE) Derbynet master has been updated to reflect this (existing master reflects the regression). am/DatabaseMetaData.java contains logic to override these to "true" when running against a >= 10.2. server. I also added code, which is presently commented out, to negotiate down (effectivly an "AND" for these boolean values) once the server can start returning the correct values. As far as I can understand, this cannot happen until we hit the next major release, 11, due to the forward compatibility requirements for client/server, cf. http://wiki.apache.org/db-derby/ForwardCompatibility . If the present solution is committed I will file a JIRA to be fixed for Derby 11 (not possible yet?) , to move to the new regime from Derby 11.0. Testing: Ran derbyall on 10.1 trunk with the extended 10.1 metadata test (derby1252-10.1.diff): derbyall/derbynetmats/derbynetmats.fail:derbynet/DerbyNetAutoStart.java derbyall/derbyall.fail:unit/T_Diagnosticable.unit This is unrelated. Ran derbyall on 10.2 trunk with the patch (derby1252.diff): wisconsin fails for all clients, plus store/TransactionTable.sql (same as tinderbox). Ran 10.1 client and extended 10.1 metadata test against current 10.2 to show regression: Ok, shows regression. Ran JCC client and extended 10.1 metadata test against current 10.2 to show regression: Ok, shows regression. Ran 10.1 client and extended 10.1 metadata test against patched 10.2 to show regression has gone away: OK. Ran JCC client and extended 10.1 metadata test against patched 10.2 to show regression has gone away: OK. Ran patched 10.2 client against 10.1 server and and extended 10.1 metadata test to see that the patched 10.2 client downgrades correctly: OK The patch is ready for review.
          Hide
          Dag H. Wanvik added a comment -

          whitespace fix for derby1252.diff

          Show
          Dag H. Wanvik added a comment - whitespace fix for derby1252.diff
          Hide
          Rick Hillegas added a comment -

          Thanks, Dag. The fix looks solid. Derbyall had two diffs for me: 1) the wisconsin noise, 2) XATest under DerbyNetClient (DERBY-1640). Committed at subversion revision 428510.

          Show
          Rick Hillegas added a comment - Thanks, Dag. The fix looks solid. Derbyall had two diffs for me: 1) the wisconsin noise, 2) XATest under DerbyNetClient ( DERBY-1640 ). Committed at subversion revision 428510.
          Hide
          Kathey Marsden added a comment -

          now that this is fixed , is there still existing application impact? If not then we should uncheck the box.
          "Existing application impact" should a field users can query on to see if there have been changes that might affect their application. Once a regression is fixed it can be unchecked if there is nothing that remains that might impact users.

          Show
          Kathey Marsden added a comment - now that this is fixed , is there still existing application impact? If not then we should uncheck the box. "Existing application impact" should a field users can query on to see if there have been changes that might affect their application. Once a regression is fixed it can be unchecked if there is nothing that remains that might impact users.
          Hide
          Dag H. Wanvik added a comment -

          regression gone after fix

          Show
          Dag H. Wanvik added a comment - regression gone after fix
          Hide
          Dag H. Wanvik added a comment -

          Setting regression again, this field should reflect bug property and not be removed when
          issue is fixed, cf mail thread http://www.nabble.com/Jira-field-definition-clarification-for-Derby-tf2024859.html#a5567827

          Show
          Dag H. Wanvik added a comment - Setting regression again, this field should reflect bug property and not be removed when issue is fixed, cf mail thread http://www.nabble.com/Jira-field-definition-clarification-for-Derby-tf2024859.html#a5567827
          Hide
          Dag H. Wanvik added a comment -

          Verified on trunk.

          Show
          Dag H. Wanvik added a comment - Verified on trunk.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development