OpenJPA
  1. OpenJPA
  2. OPENJPA-2235

"READ_UNCOMMITTED" setting for the fetch plan isolation level is ignored in DB2Dictionary

    Details

    • Patch Info:
      Patch Available

      Description

      When user set query hint as shown below for db2, no "with UR" clause append to the query. The setting is ignored.
      query.setHint("openjpa.FetchPlan.Isolation", "READ_UNCOMMITTED");

      Uncommitted read is very risky and should be avoid if it is possible. JPA specification requires a minimum of read-committed isolation to ensure no "dirty read" and "non-repeatible read" can occur. Use of read-uncommitted isolation may cause data integrity problem.

        Activity

        Albert Lee made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Albert Lee made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Albert Lee made changes -
        Fix Version/s 2.1.2 [ 12317142 ]
        Fix Version/s 2.3.0 [ 12319463 ]
        Fix Version/s 2.2.2 [ 12323356 ]
        Fix Version/s 2.2.1.1 [ 12323484 ]
        Albert Lee made changes -
        Patch Info Patch Available [ 10042 ]
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12541605 ]
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12537360 ]
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12538028 ]
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12541604 ]
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12541604 ]
        Hide
        Helen Xu added a comment -

        We'll have to change the interface from the abstract class and all the subclasses if we want to break the function. It is kind of risky.

        I just added a bit more comment to the method to indicate it also handle the UR hint.

        Show
        Helen Xu added a comment - We'll have to change the interface from the abstract class and all the subclasses if we want to break the function. It is kind of risky. I just added a bit more comment to the method to indicate it also handle the UR hint.
        Hide
        Kevin Sutter added a comment -

        The code patch looks clean. I would suggest a few more comments in the header block for this method... Since we're kind of extending the use of the getForUpdateClause() method to only do the "WITH UR" clause if isForUpdate parameter is false, it would be good to explain that. Either that, or we should consider breaking off these two aspects into separate methods. Not sure of the ripple effect though with this type of change. Thanks.

        Show
        Kevin Sutter added a comment - The code patch looks clean. I would suggest a few more comments in the header block for this method... Since we're kind of extending the use of the getForUpdateClause() method to only do the "WITH UR" clause if isForUpdate parameter is false, it would be good to explain that. Either that, or we should consider breaking off these two aspects into separate methods. Not sure of the ripple effect though with this type of change. Thanks.
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12538028 ]
        Hide
        Helen Xu added a comment -

        good catch. Thanks

        we should only check the isolation hint. Modified the code for that. Attached a new patch.

        Show
        Helen Xu added a comment - good catch. Thanks we should only check the isolation hint. Modified the code for that. Attached a new patch.
        Hide
        Kevin Sutter added a comment -

        Helen,
        Thanks for taking a look at this jira and providing a patch. Based on the initial problem description, we're only concerned with the openjpa.FetchPlan.Isolation hint getting set to UR, right? But, your code patch would also work if the openjpa.jdbc.TransactionIsolation property is set. At least that's what I get from this conditional:

        if ( isolationLevel == Connection.TRANSACTION_READ_UNCOMMITTED ) {

        and isolation level is set via these statements earlier in this method:

        if (fetch != null && fetch.getIsolation() != -1)
        isolationLevel = fetch.getIsolation();
        else
        isolationLevel = conf.getTransactionIsolationConstant();

        So, if we're looking to support UR for both the FetchPlan hint and the TransactionIsolation property, then your patch looks good. But, if we're trying to fix just the FetchPlan hint, then the patch might be too broad.

        Show
        Kevin Sutter added a comment - Helen, Thanks for taking a look at this jira and providing a patch. Based on the initial problem description, we're only concerned with the openjpa.FetchPlan.Isolation hint getting set to UR, right? But, your code patch would also work if the openjpa.jdbc.TransactionIsolation property is set. At least that's what I get from this conditional: if ( isolationLevel == Connection.TRANSACTION_READ_UNCOMMITTED ) { and isolation level is set via these statements earlier in this method: if (fetch != null && fetch.getIsolation() != -1) isolationLevel = fetch.getIsolation(); else isolationLevel = conf.getTransactionIsolationConstant(); So, if we're looking to support UR for both the FetchPlan hint and the TransactionIsolation property, then your patch looks good. But, if we're trying to fix just the FetchPlan hint, then the patch might be too broad.
        Helen Xu made changes -
        Attachment OPENJPA-2235.patch [ 12537360 ]
        Helen Xu made changes -
        Attachment OPENJPA-1955.patch [ 12537267 ]
        Helen Xu made changes -
        Field Original Value New Value
        Attachment OPENJPA-1955.patch [ 12537267 ]
        Hide
        Helen Xu added a comment -

        Fix and unit test attached.

        Show
        Helen Xu added a comment - Fix and unit test attached.
        Helen Xu created issue -

          People

          • Assignee:
            Helen Xu
            Reporter:
            Helen Xu
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development