Derby
  1. Derby
  2. DERBY-4031

If NO_GENERATED_KEYS is specified, Statement.getGeneratedKeys should return an empty ResultSet, not null

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 10.3.3.0, 10.4.2.0, 10.5.1.1
    • Fix Version/s: None
    • Component/s: JDBC
    • Urgency:
      Normal
    • Issue & fix info:
      Release Note Needed
    • Bug behavior facts:
      Deviation from standard

      Description

      Currently Derby returns null from Statement.getGeneratedKeys() if the statement is prepared or executed with Statement.NO_GENERATED_KEYS. The javadoc says:

      If this Statement object did not generate any keys, an empty ResultSet object is returned.

      so we should return an empty ResultSet for this case. It is not entirely clear what the structure of the ResultSet and ResultSetMetaData should be. I tend to think the following:

      getConcurrency() - inherit from the parent statement
      getStatement() - null

      ResultSetMetaData
      getColumnCount - 0

      an alternative would be to have the ResultSetMetaData match the signature of Identity_Val_local() but I think to return 0 columns makes more sense.

        Issue Links

          Activity

          Kathey Marsden created issue -
          Kathey Marsden made changes -
          Field Original Value New Value
          Link This issue relates to DERBY-4015 [ DERBY-4015 ]
          Hide
          Kathey Marsden added a comment -

          See DERBY-4015 for discussion on this issue. Fixing this issue should resolve DERBY-4015

          Show
          Kathey Marsden added a comment - See DERBY-4015 for discussion on this issue. Fixing this issue should resolve DERBY-4015
          Hide
          Mamta A. Satoor added a comment -

          I have no standards to backup my input here but I think we should always return a ResultSet whose metadata matches the signature of identity_Val_local(). In other words, whether we are dealing with
          1)Statement.getGeneratedKeys() with Statement.RETURN_GENERATED_KEYS but the statement does not generate any keys
          2)Statement.getGeneratedKeys() with Statement.NO_GENERATED_KEYS
          we should return a consistent ResultSet object where the metadata stays the same.

          Show
          Mamta A. Satoor added a comment - I have no standards to backup my input here but I think we should always return a ResultSet whose metadata matches the signature of identity_Val_local(). In other words, whether we are dealing with 1)Statement.getGeneratedKeys() with Statement.RETURN_GENERATED_KEYS but the statement does not generate any keys 2)Statement.getGeneratedKeys() with Statement.NO_GENERATED_KEYS we should return a consistent ResultSet object where the metadata stays the same.
          Hide
          Knut Anders Hatlen added a comment -

          Matching IDENTITY_VAL_LOCAL() means that we always return a ResultSet with one column, but it is possible to have multiple generated key columns, so I'm not sure if matching the signature of IDENTITY_VAL_LOCAL() makes more sense than having 0 columns. Derby doesn't support multiple identity columns yet, but there is an API to support it.

          I think I would have preferred that an exception was thrown, though. The justification for the empty ResultSet appears to be this sentence in the javadoc:

          > If this Statement object did not generate any keys, an empty ResultSet object is returned.

          But in the case we're discussing, it's not necessarily the case that no keys were generated by the Statement object, it's just that we said that we didn't want the auto-generated keys to be returned.

          The way I understand the javadoc, it is acceptable to throw an exception in this case. In particular, it says:

          > Throws:
          > SQLException - if a database access error occurs or this method is called on a closed Statement

          So I think we could throw an SQLException because we failed to get the generated keys from the database because we had specified that we didn't want the generated keys.

          Show
          Knut Anders Hatlen added a comment - Matching IDENTITY_VAL_LOCAL() means that we always return a ResultSet with one column, but it is possible to have multiple generated key columns, so I'm not sure if matching the signature of IDENTITY_VAL_LOCAL() makes more sense than having 0 columns. Derby doesn't support multiple identity columns yet, but there is an API to support it. I think I would have preferred that an exception was thrown, though. The justification for the empty ResultSet appears to be this sentence in the javadoc: > If this Statement object did not generate any keys, an empty ResultSet object is returned. But in the case we're discussing, it's not necessarily the case that no keys were generated by the Statement object, it's just that we said that we didn't want the auto-generated keys to be returned. The way I understand the javadoc, it is acceptable to throw an exception in this case. In particular, it says: > Throws: > SQLException - if a database access error occurs or this method is called on a closed Statement So I think we could throw an SQLException because we failed to get the generated keys from the database because we had specified that we didn't want the generated keys.
          Hide
          Kathey Marsden added a comment -

          Thanks Knut for the input. Do you think we should throw an exception for the emptyArray and nullParam cases too, since we currently treat those as NO_KEYS_GENERATED or do you think we should do something else for those cases?

          Show
          Kathey Marsden added a comment - Thanks Knut for the input. Do you think we should throw an exception for the emptyArray and nullParam cases too, since we currently treat those as NO_KEYS_GENERATED or do you think we should do something else for those cases?
          Hide
          Knut Anders Hatlen added a comment -

          What I would have expected was that at least nullParam would throw an exception, since the javadoc says that the parameter should be an array, and null is not an array.

          The returned ResultSet normally has as many columns as there are elements in the array. So if we don't throw an exception for emptyArray, the consistent way to handle it would be to return a ResultSet with 0 columns and the same number of rows as the number of generated keys. I don't see how this could be useful, though, so it might be better to reject it with an error like "Auto-generated keys must have at least one column" or something.

          Show
          Knut Anders Hatlen added a comment - What I would have expected was that at least nullParam would throw an exception, since the javadoc says that the parameter should be an array, and null is not an array. The returned ResultSet normally has as many columns as there are elements in the array. So if we don't throw an exception for emptyArray, the consistent way to handle it would be to return a ResultSet with 0 columns and the same number of rows as the number of generated keys. I don't see how this could be useful, though, so it might be better to reject it with an error like "Auto-generated keys must have at least one column" or something.
          Myrna van Lunteren made changes -
          Fix Version/s 10.5.0.0 [ 12313010 ]
          Myrna van Lunteren made changes -
          Affects Version/s 10.5.1.1 [ 12313771 ]
          Affects Version/s 10.5.0.0 [ 12313010 ]
          Dag H. Wanvik made changes -
          Issue & fix info [Existing Application Impact] [Release Note Needed]
          Hide
          Knut Anders Hatlen added a comment -

          (Removed lots of trailing blank lines from the description for readability.)

          Show
          Knut Anders Hatlen added a comment - (Removed lots of trailing blank lines from the description for readability.)
          Knut Anders Hatlen made changes -
          Description Currently Derby returns null from Statement.getGeneratedKeys() if the statement is prepared or executed with Statement.NO_GENERATED_KEYS. The javadoc says:

          If this Statement object did not generate any keys, an empty ResultSet object is returned.

          so we should return an empty ResultSet for this case. It is not entirely clear what the structure of the ResultSet and ResultSetMetaData should be. I tend to think the following:

          getConcurrency() - inherit from the parent statement
          getStatement() - null

          ResultSetMetaData
          getColumnCount - 0


          an alternative would be to have the ResultSetMetaData match the signature of Identity_Val_local() but I think to return 0 columns makes more sense.


































          Currently Derby returns null from Statement.getGeneratedKeys() if the statement is prepared or executed with Statement.NO_GENERATED_KEYS. The javadoc says:

          If this Statement object did not generate any keys, an empty ResultSet object is returned.

          so we should return an empty ResultSet for this case. It is not entirely clear what the structure of the ResultSet and ResultSetMetaData should be. I tend to think the following:

          getConcurrency() - inherit from the parent statement
          getStatement() - null

          ResultSetMetaData
          getColumnCount - 0


          an alternative would be to have the ResultSetMetaData match the signature of Identity_Val_local() but I think to return 0 columns makes more sense.
          Hide
          Knut Anders Hatlen added a comment -

          Triaged for 10.5.2.

          Show
          Knut Anders Hatlen added a comment - Triaged for 10.5.2.
          Knut Anders Hatlen made changes -
          Urgency Normal
          Kathey Marsden made changes -
          Labels derby_triage10_5_2
          Gavin made changes -
          Workflow jira [ 12451212 ] Default workflow, editable Closed status [ 12801647 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Kathey Marsden
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development