Derby
  1. Derby
  2. DERBY-4767

Detailed prompt for Error XCL16 is different between Client and Embed

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.7.1.1
    • Component/s: JDBC
    • Labels:
      None
    • Bug behavior facts:
      Embedded/Client difference

      Description

      For the sql script below:

      create table t1(c11 int, c12 int);

      – insert data into tables
      insert into t1 values(1,1);
      insert into t1 values(2,2);

      – set autocommit off
      autocommit off;

      get with nohold cursor jdk1 as 'SELECT * FROM t1';

      – do fetches from the cursor
      next jdk1;

      --commit
      commit;

      – now try the fetch on cursor again after commit
      – cursors jdk1 will give Error XCL16
      next jdk1;

      – clean up.
      close jdk1;

      for the line "next jdk1;", an Error XCL16 will be thrown. However, detailed prompt for Error XCL16 is different between Client and Embed.
      In client mode, we get "ERROR XCL16: ResultSet not open. Verify that autocommit is OFF."
      While, in embed mode, we get "ERROR XCL16: ResultSet not open. Operation 'next' not permitted. Verify that autocommit is OFF."

      1. DERBY-4767.diff3
        23 kB
        Myrna van Lunteren
      2. DERBY-4767.stat3
        0.2 kB
        Myrna van Lunteren
      3. derby-4767-1.stat
        2 kB
        Yun Lee
      4. derby-4767-1.patch
        62 kB
        Yun Lee
      5. derby-4767.stat
        0.6 kB
        Yun Lee
      6. derby-4767.patch
        33 kB
        Yun Lee

        Issue Links

          Activity

          Hide
          Myrna van Lunteren added a comment -

          Committed the patch to correct the findColumnX entries in client.am.ResultSet & the 1 occurrence in impl.jdbc.EmbedResultSet and the check added to functionTests.tests.jdbcapi.ClosedObjectTest with revision 986145.
          Closing again.

          Show
          Myrna van Lunteren added a comment - Committed the patch to correct the findColumnX entries in client.am.ResultSet & the 1 occurrence in impl.jdbc.EmbedResultSet and the check added to functionTests.tests.jdbcapi.ClosedObjectTest with revision 986145. Closing again.
          Hide
          Myrna van Lunteren added a comment -

          I decided to add anoter check to one more test - jdbcapi.ClosedObjectTest - and it showed that the findColumnX method in client/am/ResultSet was called not just from findColumn, but a number of other methods. So, passing on 'findColumn' was not the right thing to do there. Instead, I'm adding a parameter to the method and passing in the appropriate operation for each time the findColumnX method is called.

          Also, this added check in ClosedObjectTest showed one more incorrect string in code not touched by the original patch, in impl.jdbc.EmbedResultSet, method rowDeleted(), the string passed on to the LANG_RESULT_SET_NOT_OPEN is "rowUpdated".

          I'll fix up these issues, run tests and if this shows no further trouble, I'll commit.

          Show
          Myrna van Lunteren added a comment - I decided to add anoter check to one more test - jdbcapi.ClosedObjectTest - and it showed that the findColumnX method in client/am/ResultSet was called not just from findColumn, but a number of other methods. So, passing on 'findColumn' was not the right thing to do there. Instead, I'm adding a parameter to the method and passing in the appropriate operation for each time the findColumnX method is called. Also, this added check in ClosedObjectTest showed one more incorrect string in code not touched by the original patch, in impl.jdbc.EmbedResultSet, method rowDeleted(), the string passed on to the LANG_RESULT_SET_NOT_OPEN is "rowUpdated". I'll fix up these issues, run tests and if this shows no further trouble, I'll commit.
          Hide
          Yun Lee added a comment -

          Thanks, Myrna. I have also created DERBY-4776 for ScrollInsensitiveResultSet.getPreviousRow() and submitted a patch.

          Show
          Yun Lee added a comment - Thanks, Myrna. I have also created DERBY-4776 for ScrollInsensitiveResultSet.getPreviousRow() and submitted a patch.
          Hide
          Myrna van Lunteren added a comment -

          committed the change I mentioned above - passing in 'update' instead of 'next' in the getPositionedUpdateResultSet method in client.am.sectionManager with revision 985613.

          Show
          Myrna van Lunteren added a comment - committed the change I mentioned above - passing in 'update' instead of 'next' in the getPositionedUpdateResultSet method in client.am.sectionManager with revision 985613.
          Hide
          Myrna van Lunteren added a comment -

          Thanks for the revised patch.

          I reviewed it, and made some minor adjustments:

          • client.am.ResultSet.java
          • checkGetterPreconditions(), you passed on the hardcoded string "operation" instead of the parameter passed in to the method. I changed it, I'm sure this was a typo
          • method lastX() is private - the operation would be "last" not "lastX".
          • method previousX() is private - the operation would be "previous"
          • relativeX - similarly, should be "relative"
          • getRowX - "getRow"
          • rather than the small test you added which would result in another master, I've added a couple of checks for the appropriate operation string to some existing tests:
            M java\testing\org\apache\derbyTesting\functionTests\tests\lang\UpdatableResultSetTest.java
            M java\testing\org\apache\derbyTesting\functionTests\tests\jdbc4\TestDbMetaData.java
            M java\testing\org\apache\derbyTesting\functionTests\tests\jdbc4\ResultSetTest.java
            M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\ResultSetMiscTest.java

          I committed the change with revision 985550.

          I have two final comments:

          • I noticed that you made the operation in client.am.sectionManager.getPositionedUpdateResultSet "next", but I think this should be "update". I'm rerunning tests with that minor change, and will commit that too if no failures arise.

          And, not relating to your change, but in looking through all the occurrences of LANG_RESULT_SET_NOT_OPEN I noticed in impl.sql.execute.ScrollInsensitiveResultSet.getPreviousRow() that the operation passed in is "next". I'm idly wondering if that perhaps should be "previous". We should ask the community and/or log a tiny bug for that.

          Show
          Myrna van Lunteren added a comment - Thanks for the revised patch. I reviewed it, and made some minor adjustments: client.am.ResultSet.java checkGetterPreconditions(), you passed on the hardcoded string "operation" instead of the parameter passed in to the method. I changed it, I'm sure this was a typo method lastX() is private - the operation would be "last" not "lastX". method previousX() is private - the operation would be "previous" relativeX - similarly, should be "relative" getRowX - "getRow" rather than the small test you added which would result in another master, I've added a couple of checks for the appropriate operation string to some existing tests: M java\testing\org\apache\derbyTesting\functionTests\tests\lang\UpdatableResultSetTest.java M java\testing\org\apache\derbyTesting\functionTests\tests\jdbc4\TestDbMetaData.java M java\testing\org\apache\derbyTesting\functionTests\tests\jdbc4\ResultSetTest.java M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\ResultSetMiscTest.java I committed the change with revision 985550. I have two final comments: I noticed that you made the operation in client.am.sectionManager.getPositionedUpdateResultSet "next", but I think this should be "update". I'm rerunning tests with that minor change, and will commit that too if no failures arise. And, not relating to your change, but in looking through all the occurrences of LANG_RESULT_SET_NOT_OPEN I noticed in impl.sql.execute.ScrollInsensitiveResultSet.getPreviousRow() that the operation passed in is "next". I'm idly wondering if that perhaps should be "previous". We should ask the community and/or log a tiny bug for that.
          Hide
          Yun Lee added a comment -

          Hi, Myrna. I have removed CLIENT_RESULT_SET_NOT_OPEN and "XCL16.S.1", just keeped "XCL16.S", please check the new patch.

          Show
          Yun Lee added a comment - Hi, Myrna. I have removed CLIENT_RESULT_SET_NOT_OPEN and "XCL16.S.1", just keeped "XCL16.S", please check the new patch.
          Hide
          Myrna van Lunteren added a comment -

          You're correct - once the CLIENT_RESULT_SET_NOT_OPEN message is no longer needed, it also needs to be removed from SQLState.java.
          And, as such, also from the related message.xml and for completeness' sake, also from the related translation files.

          I will look at the current patch today.

          Show
          Myrna van Lunteren added a comment - You're correct - once the CLIENT_RESULT_SET_NOT_OPEN message is no longer needed, it also needs to be removed from SQLState.java. And, as such, also from the related message.xml and for completeness' sake, also from the related translation files. I will look at the current patch today.
          Hide
          Yun Lee added a comment -

          Hi, Myrna. I have replaced CLIENT_RESULT_SET_NOT_OPEN by LANG_RESULT_SET_NOT_OPEN with the suitable params and cretaed a testxcl16.sql to test this issue. Please check the patch.

          Besides, I have a question, CLIENT_RESULT_SET_NOT_OPEN is not used anywhere now, except in SQLState.java, why not deleted it and change "XCL16.S.0" to "XCL16.S"?

          Show
          Yun Lee added a comment - Hi, Myrna. I have replaced CLIENT_RESULT_SET_NOT_OPEN by LANG_RESULT_SET_NOT_OPEN with the suitable params and cretaed a testxcl16.sql to test this issue. Please check the patch. Besides, I have a question, CLIENT_RESULT_SET_NOT_OPEN is not used anywhere now, except in SQLState.java, why not deleted it and change "XCL16.S.0" to "XCL16.S"?
          Hide
          Myrna van Lunteren added a comment -

          The why is probably, because the client came from a different background than embedded.

          In DERBY-3801 I mentioned I took a quick look and thought it was possible to pass on the operation that caused the CLIENT_RESULT_SET_NOT_OPEN to be passed on. But I'm not a client/server/drda expert, so perhaps I'm wrong.

          To pull these two messages together, without losing helpful information, the thing to do would be to add the information about the operation causing the error situation to be propogated.

          You cannot simply replace CLIENT_RESULT_SET_NOT_OPEN by LANG_RESULT_SET_NOT_OPEN, because the LANG one has an additional parameter, that's not available right now for the CLIENT one. You'd get NPEs at worst, empty strings at best. So, some work is involved in finding what the operation is in each call for the CLIENT one.

          As the CLIENT one seems to be called from one of two methods (client.am.ResultSet.checkForClosedResultSet(), and client.am.SectionManager()), those methods will need to be changed to pass the operation through. Judging by the usage of LANG_RESULT_SET_NOT_OPEN, depending on the situation, either variables for the operation strings (like they're defined for the LANG message in e.g. impl.sql.execute.BasicNoPutResultSetImpl.java) will have to get defined somewhere (I don't know the best place for that, but I believe the client code has a central place for strings like that), or they can be hardcoded/ad hoc (like e.g. in MaterializedResultSet).

          (note, the operations are commands, not language constructs/messages, so don't need to be translated, and thus, can be hardcoded).

          Show
          Myrna van Lunteren added a comment - The why is probably, because the client came from a different background than embedded. In DERBY-3801 I mentioned I took a quick look and thought it was possible to pass on the operation that caused the CLIENT_RESULT_SET_NOT_OPEN to be passed on. But I'm not a client/server/drda expert, so perhaps I'm wrong. To pull these two messages together, without losing helpful information, the thing to do would be to add the information about the operation causing the error situation to be propogated. You cannot simply replace CLIENT_RESULT_SET_NOT_OPEN by LANG_RESULT_SET_NOT_OPEN, because the LANG one has an additional parameter, that's not available right now for the CLIENT one. You'd get NPEs at worst, empty strings at best. So, some work is involved in finding what the operation is in each call for the CLIENT one. As the CLIENT one seems to be called from one of two methods (client.am.ResultSet.checkForClosedResultSet(), and client.am.SectionManager()), those methods will need to be changed to pass the operation through. Judging by the usage of LANG_RESULT_SET_NOT_OPEN, depending on the situation, either variables for the operation strings (like they're defined for the LANG message in e.g. impl.sql.execute.BasicNoPutResultSetImpl.java) will have to get defined somewhere (I don't know the best place for that, but I believe the client code has a central place for strings like that), or they can be hardcoded/ad hoc (like e.g. in MaterializedResultSet). (note, the operations are commands, not language constructs/messages, so don't need to be translated, and thus, can be hardcoded).
          Hide
          Yun Lee added a comment -

          I have a question about this. When I need change Derby client code to match behavior with Embedded driver, does it mean that for this sql script, CLIENT_RESULT_SET_NOT_OPEN(XCL16.S.1) should be replaced by LANG_RESULT_SET_NOT_OPEN(XCL16.S.0)? And then, why did DERBY created two different(but very similiar) messages?

          Show
          Yun Lee added a comment - I have a question about this. When I need change Derby client code to match behavior with Embedded driver, does it mean that for this sql script, CLIENT_RESULT_SET_NOT_OPEN(XCL16.S.1) should be replaced by LANG_RESULT_SET_NOT_OPEN(XCL16.S.0)? And then, why did DERBY created two different(but very similiar) messages?

            People

            • Assignee:
              Yun Lee
              Reporter:
              Yun Lee
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development