Issue Details (XML | Word | Printable)

Key: OPENJPA-182
Type: New Feature New Feature
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: David Wisneski
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
OpenJPA

db2 update lock syntax WITH <isolation> USE AND KEEP UPDATE LOCKS

Created: 26/Mar/07 05:22 PM   Updated: 18/Apr/07 08:22 PM
Return to search
Component/s: jdbc
Affects Version/s: None
Fix Version/s: 0.9.7

Time Tracking:
Not Specified

File Attachments:
  Size
Text File JIRA182-subselect.patch 2007-04-17 09:43 PM Ritika Maheshwari 3 kB
Text File Licensed for inclusion in ASF works OPENJPA-182.patch 2007-04-06 06:45 PM Patrick Linskey 4 kB
Text File Licensed for inclusion in ASF works OPENJPA-182.patch 2007-04-04 11:49 PM Patrick Linskey 14 kB
Text File openJPA182.patch 2007-03-28 09:58 PM Ritika Maheshwari 13 kB
Java Archive File openjpa182TestCase.jar 2007-04-05 09:06 PM Ritika Maheshwari 1 kB
Environment: db2 database driver for zOS, AS400, Unix, Windows, Linux
Issue Links:
Reference
 

Resolution Date: 06/Apr/07 11:54 PM


 Description  « Hide
A while back we changed the syntax of update locking from FOR UPDATE OF to WITH RS USE AND KEEP UPDATE LOCKS. Additional changes are required because
1. if isolation=serializable is configured, then the syntax should be WITH RR USE AND KEEP UDPATE LOCKS
2. when using DB2/400 on iSeries machines, the syntax is WITH RS USE AND KEEP EXCLUSIVE LOCKS or WITH RR USE AND KEEP EXCLUSIVE LOCKS because DB2/400 only supports read or exclusive locks.
3. DB2 supports both a FETCH FIRST ROWS and update LOCKS clauses.

So we change supportsLockingWithSelectRange = true in the AbstractDB2Dictionary class and change the DB2Dictionary to append the correct LOCKS syntax depending on vendor, release and isolation level.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Ritika Maheshwari added a comment - 28/Mar/07 09:58 PM
here is a patch for this feature

Ritika Maheshwari made changes - 28/Mar/07 09:58 PM
Field Original Value New Value
Attachment openJPA182.patch [ 12354468 ]
Patrick Linskey added a comment - 29/Mar/07 08:15 AM
Wow... that's a lot more DB2 locking knowledge than I've ever seen in one place. Neat.

Some comments on the patch:

1. How does the openjpa.hint.updateClause hint differ from the value of the forUpdate flag passed in to the DBDictionary call? It looks like the existing OpenJPA per-transaction read / write lock level configuration could be used instead.

2. Is openjpa.hint.isolationLevel really a hint, or more of a rule? Again, I have a hunch that maybe we could do something with the read / write lock levels, or maybe some other means of controlling isolation level. In any event, it seems like isolation level isn't really a hint, but rather is more of a rule.

3. You introduced a number of public boolean fields for determining what type of DB2 instance is being used. Based on code inspection, it looks like you expect that it should always be possible to automatically determine the type; maybe these should be private fields instead? We only have public fields in DBDictionaries for user-configurable settings. Also, since it looks like only one of the booleans can meaningfully be true, I'd rather see a single private db2ServerType field that will be set to one of several symbolic constant values. This will let you replace the if-else block with a switch block if you prefer that sort of thing, also.

Ritika Maheshwari added a comment - 29/Mar/07 07:20 PM
1. The updateFlag is set to true when the JPA query is running in pessimistic mode.Where as with hint even when u are in optimistic mode you can get the update flags.This way user can use the standard JPA api setHint and get the lock clause.Where as in the other case they need to use OpenJPA specific api.Besides hint is per query basis.We also needed the hints for Static SQL and Named Queries.

2. I do not understand what u mean by rule versus hint.Isolation level hint tells us whther to add WITH RR or WITH RS.And again this is per query basis where as the isolation hint through the persistence.xml is to broad applying to all the entities.Then again we need the hints for NamedQuery and Static SQL.

3. Agreed

Repository Revision Date User Message
ASF #525252 Tue Apr 03 19:34:59 UTC 2007 wisneskid changes for JIRA OPENJPA-182
Files Changed
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractDB2Dictionary.java

Repository Revision Date User Message
ASF #525262 Tue Apr 03 20:24:39 UTC 2007 wisneskid fix to OPENJPA-182 commit
Files Changed
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java

Repository Revision Date User Message
ASF #525602 Wed Apr 04 21:02:51 UTC 2007 wisneskid removing unneeded method in OPENJPA-182 fix
Files Changed
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java

Patrick Linskey added a comment - 04/Apr/07 11:49 PM
For the sake of discussion, I've attached an alternate patch that uses a new JDBCFetchPlan.setIsolationLevel() instead of a hint for isolation level, and uses JDBCFetchConfiguration.getReadLockLevel() to determine whether or not to do a SELECT ... FOR UPDATE.

If the read lock level is set to LockLevels.LEVEL_WRITE, then the FOR UPDATE is included; if the read lock level is set to LockLevels.LEVEL_READ, then no FOR UPDATE is used. If the read lock level is LockLevels.LEVEL_NONE, then the default behavior is used. (This is possibly not the best use of LEVEL_NONE -- I'm not sure if LEVEL_NONE means 'default' or something else. But for the purposes of demonstration, it seemed expedient to use it. Adding a new LEVEL_DEFAULT constant might make more sense.)

Also, I directly reused the java.sql.Connection constants, which is possibly non-ideal; we might want to discuss making our own constants. Or not.

So, in this model, if there were a test case for this stuff, I would have changed the test case to do:

Query q = em.createQuery(...);
JDBCFetchPlan plan = (JDBCFetchPlan) ((OpenJPAQuery) query).getFetchPlan();
plan.setIsolationLevel(Connection.TRANSACTION_SERIALIZABLE);
plan.setReadLockMode(LockModeType.WRITE); // force a FOR UPDATE
List l = q.getResultList();

Note also that this model allows the isolation level and read lock mode to be configured on the EM itself, for use in find() calls and relationship lookups, and as the default settings for the fetch plans of queries created from the EM.

Finally, I replicated the logic in DB2Dictionary, but I noticed that sometimes the logic checked for "serializable" and sometimes it checked for "read-uncommitted". I preserved these checks, but was this intentional? I'm not all that clear about what the optimizations are, so just wanted to ensure that that wasn't a typo.

Patrick Linskey made changes - 04/Apr/07 11:49 PM
Attachment OPENJPA-182.patch [ 12354964 ]
Craig Russell added a comment - 04/Apr/07 11:59 PM
A use-case for the isolation level is to support different locking semantics for different classes and possibly for different queries. To use Patrick's patch would I need to use a different fetch plan before issuing a find or a query and then set it back after that method call? Or can I specify an isolation level in the FetchConfiguration per class?

Patrick Linskey added a comment - 05/Apr/07 12:33 AM
In the hint-based approach, you could set the hints on a particular query.

In my approach, you could configure the FetchPlan for a particular query (get the Query's fetch plan), or for a particular entity manager (get the EM's fetch plan). To do different fetch plans for different queries, you'd just set the different query fetch plans differently.

Query fetch plans are a copy-on-write semantic -- they start off just delegating through to the EM's fetch plan, and if you make changes, then they immediately make a copy of the EM's fetch plan, and remain isolated from the EM's fetch plan from then on out.

If you wanted to do different isolation levels for different find() calls or different relationship traversals, in my impl, you'd need to set the EM's fetch plan, do the work, then set it back. In the hint-based impl, you would not be able to do any configuration for find() or relationship traversals.

Patrick Linskey added a comment - 05/Apr/07 12:43 AM
It's probably worth noting that everything in the FetchPlan is configurable via the Query.setHint() APIs. My earlier theoretical test case would become:

Query q = em.createQuery(...);
q.setHint("openjpa.FetchPlan.IsolationLevel", Connection.TRANSACTION_SERIALIZABLE);
q.setHint("openjpa.FetchPlan.ReadLockMode", LockModeType.WRITE);
List l = q.getResultList();

In other words, if we use the patch that I attached (or something like it), we will end up with strong typing, API alignment with similar concepts, and dynamic accessibility both in terms of Query.setHint() and @QueryHints annotations / query-hint XML.

Abe White added a comment - 05/Apr/07 02:45 PM
What is setting the isolation level this way actually doing? For anything other than DB2 right now, it looks like it doesn't do anything. And even for DB2, it's unclear to me exactly what the isolation level hint is doing, and why the information can't be gleaned from the global setting or the connection. It seems very misleading to me to have a setIsolationLevel API (or generic "IsolationLevel" hint) that doesn't actually change the connection's isolation level.

If we can determine that this API is actually useful for more than DB2, and if we can name it appropriately for what it actually does, then I like Patrick's proposal of a FetchPlan API. The fact that all FetchPlan properties can be expressed as hints should make everyone happy. But if this is just a DB2 thing, then we should rename the hint to have a DB2-specific name and be done with it IMO.

Craig Russell added a comment - 05/Apr/07 03:09 PM
Hopefully, this will be useful for applications where there are "hot" tables that require pessimistic locking even though the rest of the application does better with optimistic. Take the example of an Order/OrderLine where there are lots of updates of the Order and/or associated OrderLine. If several threads get the same Order, they will ultimately conflict and waste time. If the query to retrieve the Order is marked as pessimistic (WRITE) then these threads will serialize and all of their work will complete.

Abe White added a comment - 05/Apr/07 03:11 PM
> Hopefully, this will be useful for applications where there are "hot" tables that require pessimistic locking even though the rest of the application does better with optimistic.

That's what our lock levels and lock APIs are for. I'm still not clear on what this is adding to the mix for most DBs.

Patrick Linskey added a comment - 05/Apr/07 04:34 PM
> > Hopefully, this will be useful for applications where there are "hot" tables
> > that require pessimistic locking even though the rest of the application
> > does better with optimistic.
>
> That's what our lock levels and lock APIs are for. I'm still not clear on what this is
> adding to the mix for most DBs.

Not really -- the lock levels allow the user to configure how locking should happen, not what the isolation level should be for the locks.

I don't know about what levels of support non-DB2 databases have for per-query isolation level configuration. Does anyone have any experience with this in other databases?

Oh, and regardless, we should change the base DBDictionary to throw an exception if this FetchPlan setting is set but not serviceable.

One thing that we should test: I'm not convinced that the lock level override in the DB2Dictionary code is necessary. It's possible that the LockManager will already take into account the current JDBCFetchConfiguration's lock level settings when specifying the forUpdate setting for the toSelect() clause. Some test cases will make it easy to figure out the answer to this question.

Craig Russell added a comment - 05/Apr/07 07:39 PM
I know that Oracle allows you to add a FOR UPDATE clause to a query, and this affects the results of that query. In Sun appserver CMP we use this to set exclusive locks on rows where we want pessimistic locking behavior just for certain tables.

Abe White added a comment - 05/Apr/07 07:44 PM
> I know that Oracle allows you to add a FOR UPDATE clause to a query, and this affects the results of that query. In Sun appserver CMP we use this to set exclusive locks on rows where we want pessimistic locking behavior just for certain tables.

Again, this is exactly what our existing lock levels and APIs do.

Patrick Linskey added a comment - 05/Apr/07 07:55 PM
> > I know that Oracle allows you to add a FOR UPDATE clause
> > to a query, and this affects the results of that query. In Sun
> > appserver CMP we use this to set exclusive locks on rows
> > where we want pessimistic locking behavior just for certain tables.
>
> Again, this is exactly what our existing lock levels and APIs do.

The current patches in this issue encompass two separate bits of functionality:

1. an update-override setting. (In the context of my patch, I'm not convinced that this is necessary, since I suspect that the code that calls toSelect() might set the forUpdate boolean based on the values in the JDBCFetchConfiguration anyways.)

2. an isolation-override setting.

Currently, this patch implements all of this only in the DB2Dictionary. As of right now, the first feature is something that is useful for all sorts of databases, via syntax like Oracle's SELECT ... FOR UPDATE. However, we only know how to implement the second feature for DB2, and not for any other database. In Oracle, "ALTER SESSION" can be used to change the isolation level of a given session, but I'm not sure of the semantics of this operation. I believe that Abe's question is: Do other databases (Sybase? Derby?) also have semantics for changing the isolation level of a particular query?

Michael Dick added a comment - 05/Apr/07 08:50 PM
I have no practical experience with Sybase, but I did find this in their TransactSQL user's guide :

"Changing the isolation level for a query

You can change the isolation level for a query by using the at isolation clause with the select or readtext statements. The at isolation clause supports isolation levels 0, 1, and 3. It does not support isolation level 2. The read uncommitted, read committed, and serializable options of at isolation represent isolation levels as listed below:

at isolation option Isolation level
read uncommited 0
read committed 1
serializable 3

For example, the following two statements query the same table at isolation levels 0 and 3, respectively:

select *
from titles
at isolation read uncommitted

select *
from titles
at isolation serializable"

There's more information online here: http://manuals.sybase.com/onlinebooks/group-as/asg1250e/sqlug/@Generic__BookTextView/53911;hf=0


Patrick Linskey added a comment - 05/Apr/07 09:02 PM
Between Ritika's SQLServer email on the dev list and Mike's Sybase research, it sounds like this feature is supported by enough databases that I think it's useful to expose as an API, rather than just a DB2-specific hint.

Ritika Maheshwari added a comment - 05/Apr/07 09:06 PM
Here is a jar containing the 2 entities on whcih I run my testcases.

em.getTransaction().begin()
Query qryA = em.createQuery("select d from DeptBean2 d where d.no = 1");
qryA.setHint("openjpa.hint.updateClause",true);
qryA.setHint("openjpa.hint.isolationLevel", "serializable");
List rsA = qryA.getResultList();

The SQL Output looks like

13109 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> executing prepstmnt 694036830 SELECT t0.no, t1.empid, t2.no, t2.mgr_empid, t2.cdname, t1.ceemp_ts, t1.cehireDate, t1.ceismanager, t1.cename, t1.cesalary, t0.cdname FROM deptab2 t0 LEFT OUTER JOIN emptab2 t1 ON t0.mgr_empid = t1.empid LEFT OUTER JOIN deptab2 t2 ON t1.dept_no = t2.no WHERE (CAST(t0.no AS BIGINT) = CAST(? AS BIGINT)) WITH RR USE AND KEEP UPDATE LOCKS [params=(long) 1]
13119 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> [10 ms] spent
27420 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> executing prepstmnt 2102295886 SELECT t0.empid, t1.no, t2.empid, t2.dept_no, t2.ceemp_ts, t2.cehireDate, t2.ceismanager, t2.cename, t2.cesalary, t1.cdname, t0.ceemp_ts, t0.cehireDate, t0.ceismanager, t0.cename, t0.cesalary FROM emptab2 t0 LEFT OUTER JOIN deptab2 t1 ON t0.dept_no = t1.no LEFT OUTER JOIN emptab2 t2 ON t1.mgr_empid = t2.empid WHERE t0.dept_no = ? WITH RR USE AND KEEP UPDATE LOCKS [params=(int) 1]
27430 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> [10 ms] spent

 Query qryB = em.createQuery("select e from EmpBean2 e where e.empid = 1");
  qryB.setHint("openjpa.hint.updateClause",true);
   qryB.setHint("openjpa.hint.isolationLevel", "read-uncommitted");
    List rsB = qryB.getResultList();

The SQL Output looks like


47969 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> executing prepstmnt 1530944320 SELECT t0.empid, t1.no, t2.empid, t2.dept_no, t2.ceemp_ts, t2.cehireDate, t2.ceismanager, t2.cename, t2.cesalary, t1.cdname, t0.ceemp_ts, t0.cehireDate, t0.ceismanager, t0.cename, t0.cesalary FROM emptab2 t0 LEFT OUTER JOIN deptab2 t1 ON t0.dept_no = t1.no LEFT OUTER JOIN emptab2 t2 ON t1.mgr_empid = t2.empid WHERE (CAST(t0.empid AS BIGINT) = CAST(? AS BIGINT)) WITH RS USE AND KEEP UPDATE LOCKS [params=(long) 1]
47969 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> [0 ms] spent

Query qryC = em.createQuery("select d from DeptBean2 d where d.no = 1");
 DeptBean2 deptC = (DeptBean2)qryC.getSingleResult();

The SQL Out put is

72695 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> executing prepstmnt 742009914 SELECT t0.no, t1.empid, t2.no, t2.mgr_empid, t2.cdname, t1.ceemp_ts, t1.cehireDate, t1.ceismanager, t1.cename, t1.cesalary, t0.cdname FROM deptab2 t0 LEFT OUTER JOIN emptab2 t1 ON t0.mgr_empid = t1.empid LEFT OUTER JOIN deptab2 t2 ON t1.dept_no = t2.no WHERE (CAST(t0.no AS BIGINT) = CAST(? AS BIGINT)) FOR READ ONLY optimize for 1 row [params=(long) 1]
72695 dwtest TRACE [main] openjpa.jdbc.SQL - <t 1094730048, conn 1593466618> [0 ms] spent

In my persistence.xml I had the following properties

  <property name="openjpa.LockManager" value="pessimistic" />
<property name="openjpa.jdbc.TransactionIsolation" value="read-committed" />

Essentially if we are running against DB2 8.2 or Later then if update lock is true then for all the isolation levels other than "serializable" WITH RS USE AND KEEP UPDATE LOCKS clause is appended to the query.In case of "serializable" isolation level "WITH RR USE AND KEEP UPDATE LOCK" is appended.

If the updateLock is false then FOR READ ONLY clause is appended to all queries.



Ritika Maheshwari made changes - 05/Apr/07 09:06 PM
Attachment openjpa182TestCase.jar [ 12355038 ]
Repository Revision Date User Message
ASF #525997 Thu Apr 05 22:55:52 UTC 2007 pcl OPENJPA-182. Moved to API-based model. Query.setHint() can still be used via the query hint => fetch plan binding.

Removed the logic to override the forUpdate value, since the calling code already incorporates fetch configuration data into its decision about how to invoke toSelect(). Added a test case to assert this behavior.

Also cleaned up some minor whitespace issues, and reduced code duplication by moving a couple of concepts up into DBDictionary. Removed some seemingly-unnecessary overrides from H2Dictionary.

Added a test case for isolation level configuration. For non-DB2 dictionaries, it asserts that an exception is thrown during execution. Someone with DB2 knowledge / access should fill in the test case for the DB2 cases. As we add support for per-query isolation level configuration for other databases, we should change this test case.
Files Changed
ADD /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties
ADD /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java
ADD /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java

Michael Dick made changes - 06/Apr/07 12:50 PM
Fix Version/s 0.9.7 [ 12312340 ]
Repository Revision Date User Message
ASF #526212 Fri Apr 06 16:08:27 UTC 2007 pcl OPENJPA-182
Files Changed
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java

Patrick Linskey added a comment - 06/Apr/07 06:45 PM
Remove forUpdate from DBDictionary.toOperation()'s signature; use a null check on forUpdateClause instead.

Patrick Linskey made changes - 06/Apr/07 06:45 PM
Attachment OPENJPA-182.patch [ 12355100 ]
Repository Revision Date User Message
ASF #526266 Fri Apr 06 19:50:53 UTC 2007 pcl OPENJPA-182. forUpdateClause is now used even if forUpdate is false, to allow for read-only optimizations. Changed JDBCFetchPlan.setIsolationLevel and JDBCFetchConfiguration.setIsolationLevel to just JDBCFetchXXX.setIsolation.
Files Changed
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java

Repository Revision Date User Message
ASF #526316 Fri Apr 06 23:22:06 UTC 2007 pcl OPENJPA-182. Changed JDBCFetchPlan.setIsolation() to use enums; added logic to handle enum hints to QueryImpl; moved from IllegalArgumentException to InvalidArgumentException to unify exception processing for both queries and find calls.
Files Changed
ADD /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/IsolationLevel.java
MODIFY /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java
MODIFY /incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java

Patrick Linskey added a comment - 06/Apr/07 11:54 PM
I think that, aside from documentation and DB2 test cases and impls for other data stores, we can put this issue to rest.

Patrick Linskey made changes - 06/Apr/07 11:54 PM
Assignee David Wisneski [ wisneskid ]
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Ritika Maheshwari added a comment - 17/Apr/07 09:43 PM
 we just discovered that there is a problem with this fix.The problem is that the FOR READ ONLY flag is getting generated for subselects as well.
 
Basically for DB2 if forUpdate was false we append FOR READ ONLY we need another check to see that it is not a subselect.To me it appears that in the toSelect method when SelectImpl is passed we check that the parent is null or not to figure out that it is a subselect and then pass another flag subselect to the getForUdateClause method where we say
    if(!forUpdate && !subselect)
         forUpdateString.append(forReadOnlyClause)
to achieve this we would probably have to override the toSelect methods in DB2Dictionary again unless there is a better way.

I am attaching a patch to specify the changes

Ritika Maheshwari made changes - 17/Apr/07 09:43 PM
Attachment JIRA182-subselect.patch [ 12355717 ]
Patrick Linskey added a comment - 17/Apr/07 10:03 PM
Some comments:

1. I don't think that we should be doing work on resolved issues. So, this should be re-opened, or (preferably) a new issue should be opened for this new bug.

2. The patch you attached does not use OpenJPA-style formatting. We don't have a style guide spelled out as well as we probably should, but we always put spaces after commas, we indent 4 spaces on continuation lines, and we put a space between an 'if' and the open paren.

3. It's a shame to have to do all this code duplication between DBDictionary and DB2Dictionary. To what extent can we refactor DBDictionary's methods to make this concept work out better for DB2Dictionary?

Michael Dick made changes - 18/Apr/07 08:22 PM
Link This issue relates to OPENJPA-222 [ OPENJPA-222 ]