Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 327940 - @OrderColumn does not work when list read in through uow
Summary: @OrderColumn does not work when list read in through uow
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Eclipselink (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Nobody - feel free to take it CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 309039 327860 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-10-15 14:33 EDT by Andrei Ilitchev CLA
Modified: 2022-06-09 10:21 EDT (History)
3 users (show)

See Also:


Attachments
Suggeste patch (14.74 KB, patch)
2010-10-15 14:56 EDT, Andrei Ilitchev CLA
no flags Details | Diff
Slightly updated patch. (16.88 KB, patch)
2010-10-15 15:02 EDT, Andrei Ilitchev CLA
no flags Details | Diff
patch - take 3. (17.52 KB, patch)
2010-10-15 15:21 EDT, Andrei Ilitchev CLA
no flags Details | Diff
patch - take 4 (24.22 KB, patch)
2010-10-15 15:39 EDT, Andrei Ilitchev CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Ilitchev CLA 2010-10-15 14:33:07 EDT
The problem happens when ordered list read in transaction through uow (as opposed to its parent session).
That's always the case when reading in transaction isolated cache entity.
Also when reading in transaction shared cache entity when uow has begun early transaction(for instance, after flush or pessimistic locking).

I am logging the bug myself to try to get the fix before today's 2.1.2 deadline.

Here's the original e-mail reporting the bug:

HI Kalpana,

   1. The docs are correct. They refer to the use case where the shared cache is turned on and if your app does not maintain ordering while mutating the list, the broken ordering is what your app would get at next find. The behavior that customer sees looks like a bug of EclipseLink to me.
   2. David Minsky from EclipseLink sustaining should help answer your second question about the sustaining process. David, we need to sync up GlassFish sustaining engineers on the process your team follows for sustaining  EclipseLink.

I am adding Andrei, a developer from EclipseLink team who implemented this feature to discuss the bug.

Hi Andrei,
I have attached a simplified test case[1] that demonstrates that a list read from db is not ordered if -<property name="eclipselink.cache.shared.default" value="false"/> is set in persistence.xml. The root cause seems to be that for queries on isolated descriptors, we use UOW instead of serversession to execute queries and then  code in ReadAllQuery [2] results in a fork in code path that results in the sorting by OrderedListContainerPolicy (addAll())  not being executed.

[1] To execute the test case, unzip the attached, adjust build.properties for your eclipselink workspace and test.properties for your database and then execute ant run (ant debug run to attach debuger)
You will observe that on retrieval from database, the list is not ordered by the value of INDEX column. If you execute the same test case after modifying src/descriptor/persitence-javase.xml to enable shared cache, you will observe that the list is ordered.

[2]  Code from ReadAllQuery.executeObjectLevelReadQuery()     
           if (this.session.isUnitOfWork()) {
                  //we go here when shared cache is not being used
                result = registerResultInUnitOfWork(rows, (UnitOfWorkImpl)this.session, this.translationRow, true);//
            } else {
                  //we go here when shared cache is being used
                result = this.containerPolicy.containerInstance(rows.size());
                this.descriptor.getObjectBuilder().buildObjectsInto(this, rows, result);
            }

Thanks,
Mitesh


On 10/13/2010 8:48 AM, Kalpana Karunamurthi wrote:
>  Hi Mitesh,
>
> I have an escalation for the following bug
> http://monaco.sfbay.sun.com/detail.jsf?cr=6977386
>
> http://docs.sun.com/app/docs/doc/821-1752/gezxw?l=en&a=view
> As per the above link, the @OrderBy works when
> eclipselink.cache.shared.default is set to false
>
> But on testing, the order is not maintained when the "eclipselink.cache.shared.default" is set to false.
> However, when "eclipselink.cache.shared.default" is set to true, the ordering is maintained.
>
> Unfortunately, the customer doesn't want caching. Hence "eclipselink.cache.shared.default" needs to be false.
>
> Now my question is :
>
> 1. Is it an oversight at the documentation end ?.
> Should it have read "eclipselink.cache.shared.default" needs to be set as true.
>
> 2. If the documentation is right, I see that the ordering is messed up when EntityManager.find(); is called.
> The implementation comes from org.eclipse.persistence.jpa.jar.  Where do we get the code from ?.
> Do we permissions to modify this code or should be file a bug ?.
>
> Can you please shed some light on this issue ?. This is quite important and urgent.
> Since the issue is on 3.0.1, I guess its applicable for 3.1 too.
>
> Thanks
> Kalpana
Comment 1 Andrei Ilitchev CLA 2010-10-15 14:56:20 EDT
Created attachment 181004 [details]
Suggeste patch

As was suggested by Mitesh, ReadAllQuery.registerResultInUnitOfWork is a culprit. The fix changes this method to follow the same pattern as in ObjectBuilder.buildObjectsInto.

Four tests were added to CacheableModelJunitTest:
  testDetailsOrder_Isolated,
  testDetailsOrder_Isolated_BeginEarlyTransaction,
  testDetailsOrder_Shared,
  testDetailsOrder_Shared_BeginEarlyTransaction
to test both isolated and shared cache cases.
Comment 2 Andrei Ilitchev CLA 2010-10-15 15:02:11 EDT
Created attachment 181005 [details]
Slightly updated patch.
Comment 3 Andrei Ilitchev CLA 2010-10-15 15:21:38 EDT
Created attachment 181007 [details]
patch - take 3.

Added two forgotten persistence.xml files for tests.
Comment 4 Andrei Ilitchev CLA 2010-10-15 15:39:28 EDT
Created attachment 181008 [details]
patch - take 4

take 3 had EntityManagerJUnitTestSuite instead of CachableModelJunitTest.
Comment 5 Andrei Ilitchev CLA 2010-10-15 16:41:08 EDT
Fixed in trunk and 2.1.2
Reviewed by Chris.
Comment 6 Frank Schwarz CLA 2010-10-16 05:49:53 EDT
*** Bug 309039 has been marked as a duplicate of this bug. ***
Comment 7 Andrei Ilitchev CLA 2010-10-18 09:37:32 EDT
*** Bug 327860 has been marked as a duplicate of this bug. ***
Comment 8 Andrei Ilitchev CLA 2010-10-26 17:28:40 EDT
Backported to 2.0.3 
Completed: At revision: 8414
Comment 9 Eclipse Webmaster CLA 2022-06-09 10:21:03 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink