Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
    1. derby-5282-3.patch
      3 kB
      Houx Zhang
    2. derby-5282-2.state
      0.5 kB
      Houx Zhang
    3. derby-5282-2.patch
      52 kB
      Houx Zhang
    4. derby-5282-1.state
      0.5 kB
      Houx Zhang
    5. derby-5282-1.patch
      52 kB
      Houx Zhang

      Activity

      Hide
      Houx Zhang added a comment -

      Ask for help. There is one test can not pass. It's testFullCoveredIndexScan().

      It says:
      junit.framework.AssertionFailedError: Unexpected row count: expected:<1> but was:<5>

      But when I changed the data from 1 to 5, It syas:
      junit.framework.AssertionFailedError: Unexpected row count: expected:<5> but was:<1>

      Thanks.

      Show
      Houx Zhang added a comment - Ask for help. There is one test can not pass. It's testFullCoveredIndexScan(). It says: junit.framework.AssertionFailedError: Unexpected row count: expected:<1> but was:<5> But when I changed the data from 1 to 5, It syas: junit.framework.AssertionFailedError: Unexpected row count: expected:<5> but was:<1> Thanks.
      Hide
      Houx Zhang added a comment -

      Just for rename the patches.

      Show
      Houx Zhang added a comment - Just for rename the patches.
      Hide
      Bryan Pendleton added a comment -

      The reason you are seeing 5 rows instead of the expected 1 row is because that particular
      test is deciding to do row-level locking instead of table-level locking, and the select from lock_table
      is returning 1 Table-level IS lock and 4 Row-level S locks.

      I ran the test with -Dderby.language.logQueryPlan=true and the choice of row-level locking
      is also confirmed by the trace output:

      Tue Jun 28 18:25:12 PDT 2011 Thread[main,5,main] (XID = 242), (SESSIONID = 3), select a from a ******* Index Scan ResultSet for A using index A_IDX at repeatable read isolation level using share row locking chosen by the optimizer
      Number of opens = 1
      Rows seen = 4
      Rows filtered = 0

      scan information:
      Bit set of columns fetched=

      {0}

      Number of columns fetched=1
      Number of deleted rows visited=0
      Number of pages visited=1
      Number of rows qualified=4
      Number of rows visited=4

      I extracted the relevant bit of SQL into a small separate file and ran it
      under IJ, and sure enough when I do that Derby chooses table-level locking.

      Tue Jun 28 18:45:21 PDT 2011 Thread[main,5,main] (XID = 245), (SESSIONID = 1), --------------------------------------------------------------------------------
      – Do full covered index scan.
      --------------------------------------------------------------------------------
      select a from a ******* Index Scan ResultSet for A using index A_IDX at serializable isolation level using share table locking chosen by the optimizer
      Number of opens = 1
      Rows seen = 4

      Then I noticed that in the row-level locking result, Derby is using "repeatable
      read" isolation, while in the table-level locking result, Derby is using "serializable"
      isolation.

      So I tried changing your test code so that instead of setting the isolation
      level to TRANSACTION_REPEATABLE_READ in the setUp() method, it
      sets TRANSACTION_SERIALIZABLE instead.

      And then the test passes – Derby uses table level locking and all is well.

      What is curious is that clearly the original test looks like it is trying to use
      REPEATABLE_READ isolation, as it contains:

      set isolation to RR;

      at the very start of the test.

      Perhaps this line in the test isn't actually working, though, and the test
      is still using SERIALIZABLE, not REPEATABLE_READ isolation?

      Can you try changing the test in your environment to use SERIALIZABLE
      isolation and verify whether or not the test then passes for you?

      Show
      Bryan Pendleton added a comment - The reason you are seeing 5 rows instead of the expected 1 row is because that particular test is deciding to do row-level locking instead of table-level locking, and the select from lock_table is returning 1 Table-level IS lock and 4 Row-level S locks. I ran the test with -Dderby.language.logQueryPlan=true and the choice of row-level locking is also confirmed by the trace output: Tue Jun 28 18:25:12 PDT 2011 Thread [main,5,main] (XID = 242), (SESSIONID = 3), select a from a ******* Index Scan ResultSet for A using index A_IDX at repeatable read isolation level using share row locking chosen by the optimizer Number of opens = 1 Rows seen = 4 Rows filtered = 0 scan information: Bit set of columns fetched= {0} Number of columns fetched=1 Number of deleted rows visited=0 Number of pages visited=1 Number of rows qualified=4 Number of rows visited=4 I extracted the relevant bit of SQL into a small separate file and ran it under IJ, and sure enough when I do that Derby chooses table-level locking. Tue Jun 28 18:45:21 PDT 2011 Thread [main,5,main] (XID = 245), (SESSIONID = 1), -------------------------------------------------------------------------------- – Do full covered index scan. -------------------------------------------------------------------------------- select a from a ******* Index Scan ResultSet for A using index A_IDX at serializable isolation level using share table locking chosen by the optimizer Number of opens = 1 Rows seen = 4 Then I noticed that in the row-level locking result, Derby is using "repeatable read" isolation, while in the table-level locking result, Derby is using "serializable" isolation. So I tried changing your test code so that instead of setting the isolation level to TRANSACTION_REPEATABLE_READ in the setUp() method, it sets TRANSACTION_SERIALIZABLE instead. And then the test passes – Derby uses table level locking and all is well. What is curious is that clearly the original test looks like it is trying to use REPEATABLE_READ isolation, as it contains: set isolation to RR; at the very start of the test. Perhaps this line in the test isn't actually working, though, and the test is still using SERIALIZABLE, not REPEATABLE_READ isolation? Can you try changing the test in your environment to use SERIALIZABLE isolation and verify whether or not the test then passes for you?
      Hide
      Knut Anders Hatlen added a comment -

      The SET ISOLATION statement follows the DB2 naming of the isolation levels, which doesn't match the names used in SQL and JDBC. And just to make things extra confusing, they both have an isolation level called "repeatable read", but not the same one, so "set isolation to RR" actually sets the isolation level to Connection.TRANSACTION_SERIALIZABLE. The mapping between DB2 isolation levels and JDBC isolation levels can be found here: http://db.apache.org/derby/docs/dev/devguide/cdevconcepts15366.html

      Show
      Knut Anders Hatlen added a comment - The SET ISOLATION statement follows the DB2 naming of the isolation levels, which doesn't match the names used in SQL and JDBC. And just to make things extra confusing, they both have an isolation level called "repeatable read", but not the same one, so "set isolation to RR" actually sets the isolation level to Connection.TRANSACTION_SERIALIZABLE. The mapping between DB2 isolation levels and JDBC isolation levels can be found here: http://db.apache.org/derby/docs/dev/devguide/cdevconcepts15366.html
      Hide
      Houx Zhang added a comment -

      Thanks, Bryan and Knut.

      It does work with the new isolation.

      Show
      Houx Zhang added a comment - Thanks, Bryan and Knut. It does work with the new isolation.
      Hide
      Bryan Pendleton added a comment -

      Excellent! And interesting to learn about the differing names for the isolation levels. I knew that once, long ago...

      I'm intending to commit this patch.

      Show
      Bryan Pendleton added a comment - Excellent! And interesting to learn about the differing names for the isolation levels. I knew that once, long ago... I'm intending to commit this patch.
      Hide
      Bryan Pendleton added a comment -

      Committed the new test to the trunk as revision 1141368.

      Thank you for your contribution to Derby!

      Show
      Bryan Pendleton added a comment - Committed the new test to the trunk as revision 1141368. Thank you for your contribution to Derby!
      Hide
      Knut Anders Hatlen added a comment -

      A couple post-commit review comments:

      • The new test should have a license header.
      • We should probably remove the try/catch in the setUp() method. If something fails in setUp(), it would be better if the test case failed. Failures that are just printed to the console are less likely to be noticed.
      • The tearDown() method should call super.tearDown() before it returns so that BaseJDBCTestCase's cleanup also runs.
      • Saving the original isolation level in setUp() and restoring it in tearDown() shouldn't be necessary since the connection isn't used again after tearDown().
      Show
      Knut Anders Hatlen added a comment - A couple post-commit review comments: The new test should have a license header. We should probably remove the try/catch in the setUp() method. If something fails in setUp(), it would be better if the test case failed. Failures that are just printed to the console are less likely to be noticed. The tearDown() method should call super.tearDown() before it returns so that BaseJDBCTestCase's cleanup also runs. Saving the original isolation level in setUp() and restoring it in tearDown() shouldn't be necessary since the connection isn't used again after tearDown().
      Hide
      Houx Zhang added a comment -

      Thanks for your advice, Knut. I have adopted them in the new patch and added a class comment. Please check it, thanks.

      Show
      Houx Zhang added a comment - Thanks for your advice, Knut. I have adopted them in the new patch and added a class comment. Please check it, thanks.
      Hide
      Bryan Pendleton added a comment -

      The new patch looks great. I'll see about committing it later today, when I get back to my main computer.

      Show
      Bryan Pendleton added a comment - The new patch looks great. I'll see about committing it later today, when I get back to my main computer.
      Hide
      Knut Anders Hatlen added a comment -

      Thanks for making the changes, Houx. Just two small nits: The class name in the license header is incorrect, and the patch adds some trailing blanks after the dropTable() call in setUp(). Otherwise, the patch looks good to me.

      Show
      Knut Anders Hatlen added a comment - Thanks for making the changes, Houx. Just two small nits: The class name in the license header is incorrect, and the patch adds some trailing blanks after the dropTable() call in setUp(). Otherwise, the patch looks good to me.
      Hide
      Bryan Pendleton added a comment -

      I corrected the class name in the header comment and cleaned up the trailing blanks, verified that the
      test continues to run properly, and submitted the follow-on patch as revision 1141769.

      Show
      Bryan Pendleton added a comment - I corrected the class name in the header comment and cleaned up the trailing blanks, verified that the test continues to run properly, and submitted the follow-on patch as revision 1141769.
      Hide
      Myrna van Lunteren added a comment -

      Dag commented (6/30/2011) on the commit of revision 1141368 - apparently replies on the commit messages don't make it back into JIRA, so just for the record, here is that comment:
      "I didn't follow this one, but I notice setUp and teardown do not call
      super.setUp, super.tearDown respectively as per the idiom. Is there a
      reason for this here? Notably BaseJDBCTestCase#tearDown does some
      cleaning up. It may not be required here, but it's generally good to
      stick to the idiom."

      Houx Zhang replied: "Yes, Dag. I agree with you, and will adopt it."

      Looks like Bryan already added super.teardown with revision 1141769.

      Show
      Myrna van Lunteren added a comment - Dag commented (6/30/2011) on the commit of revision 1141368 - apparently replies on the commit messages don't make it back into JIRA, so just for the record, here is that comment: "I didn't follow this one, but I notice setUp and teardown do not call super.setUp, super.tearDown respectively as per the idiom. Is there a reason for this here? Notably BaseJDBCTestCase#tearDown does some cleaning up. It may not be required here, but it's generally good to stick to the idiom." Houx Zhang replied: "Yes, Dag. I agree with you, and will adopt it." Looks like Bryan already added super.teardown with revision 1141769.

        People

        • Assignee:
          Houx Zhang
          Reporter:
          Houx Zhang
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development