Details

    • Type: Test Test
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1.0, 1.2.1
    • Fix Version/s: 1.2.2, 1.3.0, 2.0.0-M2
    • Component/s: jdbc
    • Labels:
      None
    • Environment:
      OS: windows XP professional
      java version 1.5.0_14
    • Patch Info:
      Patch Available

      Description

      Currently TestEJBQueryInterface is excluded.

      To enable it, just a little change is needed: invoking em.flush() after every em.persist().

      The test case wants to get the result depends on the order of the creation of the entity instance. but we (as well as most other JPA implementations) don't care about the creation order of java instances because the insertion happens when the transaction is committed or flush is invoked. So adding "em.flush()" after "em.persist()" will resolve the problem.

        Issue Links

          Activity

          Hide
          Michael Dick added a comment -

          I've enabled the testcase in all the releases that have OPENJPA-817, so I'm marking the issue as resolved.

          Amy if you'd like to ensure this testcase executes on 1.1, please re-open and I'll assign it to Abe / David Ezzio and let them take a look.

          Show
          Michael Dick added a comment - I've enabled the testcase in all the releases that have OPENJPA-817 , so I'm marking the issue as resolved. Amy if you'd like to ensure this testcase executes on 1.1, please re-open and I'll assign it to Abe / David Ezzio and let them take a look.
          Hide
          Michael Dick added a comment -

          Hi Amy,

          Patrick or Abe would be the person (people) to ask about merging OPENJPA-817 to 1.1.x - they're the release managers for that branch. It would be a change in behavior and 1.1.0 was released a long time ago, it might be a bit risky to introduce at this time.

          I'd be okay with leaving the testcase disabled too, but it's really their call.

          Show
          Michael Dick added a comment - Hi Amy, Patrick or Abe would be the person (people) to ask about merging OPENJPA-817 to 1.1.x - they're the release managers for that branch. It would be a change in behavior and 1.1.0 was released a long time ago, it might be a bit risky to introduce at this time. I'd be okay with leaving the testcase disabled too, but it's really their call.
          Hide
          Amy Yang added a comment -

          Hi Michael,
          thank you very much. Got it.

          my last question: is it feasible to merge fix of OPENJPA-817 to branch 1.1.x? if statement batching support is introduced in the 1.2.x branch, maybe it's ok to let the test case disabled in 1.1.x.

          Show
          Amy Yang added a comment - Hi Michael, thank you very much. Got it. my last question: is it feasible to merge fix of OPENJPA-817 to branch 1.1.x? if statement batching support is introduced in the 1.2.x branch, maybe it's ok to let the test case disabled in 1.1.x.
          Hide
          Michael Dick added a comment -

          Hi Amy,

          ArrayList allows duplicates, we needed an ordered collection that doesn't store the same StateManager twice.

          I'm guessing you applied the patch from trunk (BrokerImpl.java wasn't changed in 1.2.x, or 1.0.x) but was in trunk due to OPENJPA-732 (which moved from ArrayList to HashSet).

          Show
          Michael Dick added a comment - Hi Amy, ArrayList allows duplicates, we needed an ordered collection that doesn't store the same StateManager twice. I'm guessing you applied the patch from trunk (BrokerImpl.java wasn't changed in 1.2.x, or 1.0.x) but was in trunk due to OPENJPA-732 (which moved from ArrayList to HashSet).
          Hide
          Amy Yang added a comment -

          Hi Kevin & Michael,
          thank you very much for comments.
          I tried to merge fix of OPENJPA-817 to branch 1.1.x and it works fine for TestEJBQueryInterface. Anyway LinkedHashMap is utilized instead of HashMap in RowManagerImpl, etc.
          I'm not sure why the fix utilizes LinkedHashSet instead of ArrayList in BrokerImpl. Michael, is it for better performance?

          Show
          Amy Yang added a comment - Hi Kevin & Michael, thank you very much for comments. I tried to merge fix of OPENJPA-817 to branch 1.1.x and it works fine for TestEJBQueryInterface. Anyway LinkedHashMap is utilized instead of HashMap in RowManagerImpl, etc. I'm not sure why the fix utilizes LinkedHashSet instead of ArrayList in BrokerImpl. Michael, is it for better performance?
          Hide
          Michael Dick added a comment -

          I'm guessing it's actually OPENJPA-817 which preserves the order of statements when using constraint update manager. Absent any other information to the contrary we'll execute in the order we were called (deletes, updates, and inserts are grouped - but it's still better than no ordering).

          Show
          Michael Dick added a comment - I'm guessing it's actually OPENJPA-817 which preserves the order of statements when using constraint update manager. Absent any other information to the contrary we'll execute in the order we were called (deletes, updates, and inserts are grouped - but it's still better than no ordering).
          Hide
          Kevin Sutter added a comment -

          I wonder if this isn't related to the statement batching support introduced in the 1.2.x branch? As Mike has pointed out, this works on everything past 1.2.x. And, the statement batching support definitely affects the order of the statements being pushed out to the database.

          Although putting the em.flush() into the testcase may resolve this issue on 1.1.x, we would be implicitly disabling statement batching since now we're pushing out every insert to the database when first persisted. This might accidentally cover up other useful test scenarios. Of couse, if we just limit this testcase update to the 1.1.x branch, then that would be safer. But, I don't like to do that just to get a testcase to pass.

          Instead of just modifying the testcase, I think the root cause of the problem on 1.1.x needs to be determined. If, after that investigation, it's determined that the testcase is incorrect and needs to changed, then we have the proper background and reasons.

          End of my two cents worth...
          Kevin

          Show
          Kevin Sutter added a comment - I wonder if this isn't related to the statement batching support introduced in the 1.2.x branch? As Mike has pointed out, this works on everything past 1.2.x. And, the statement batching support definitely affects the order of the statements being pushed out to the database. Although putting the em.flush() into the testcase may resolve this issue on 1.1.x, we would be implicitly disabling statement batching since now we're pushing out every insert to the database when first persisted. This might accidentally cover up other useful test scenarios. Of couse, if we just limit this testcase update to the 1.1.x branch, then that would be safer. But, I don't like to do that just to get a testcase to pass. Instead of just modifying the testcase, I think the root cause of the problem on 1.1.x needs to be determined. If, after that investigation, it's determined that the testcase is incorrect and needs to changed, then we have the proper background and reasons. End of my two cents worth... Kevin
          Hide
          Amy Yang added a comment -

          Hi Michael,
          The error on 1.1.x is like:
          Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 18.453 sec <<< FAILURE!
          testSetFirstResults(org.apache.openjpa.persistence.jpql.clauses.TestEJBQueryInterface) Time elapsed: 0.359 sec <<< FAILURE!
          junit.framework.ComparisonFailure: expected:<...3> but was:<...0>
          at junit.framework.Assert.assertEquals(Assert.java:81)
          at junit.framework.Assert.assertEquals(Assert.java:87)
          at org.apache.openjpa.persistence.jpql.clauses.TestEJBQueryInterface.testSetFirstResults(TestEJBQueryInterface.java:124)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:585)
          at junit.framework.TestCase.runTest(TestCase.java:154)
          at junit.framework.TestCase.runBare(TestCase.java:127)
          at junit.framework.TestResult$1.protect(TestResult.java:106)
          at junit.framework.TestResult.runProtected(TestResult.java:124)
          at junit.framework.TestResult.run(TestResult.java:109)
          at junit.framework.TestCase.run(TestCase.java:118)
          at org.apache.openjpa.persistence.test.PersistenceTestCase.run(PersistenceTestCase.java:127)
          at junit.framework.TestSuite.runTest(TestSuite.java:208)
          at junit.framework.TestSuite.run(TestSuite.java:203)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:585)
          at org.apache.maven.surefire.junit.JUnitTestSet.execute(JUnitTestSet.java:213)
          at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:140)
          at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:127)
          at org.apache.maven.surefire.Surefire.run(Surefire.java:177)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:585)
          at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:334)
          at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:980)

          the insertion order seems random so the error changes every time.

          I also tried on trunk but can't reproduce the error. Do we maintain the order of the creation of java instances somewhere?

          Show
          Amy Yang added a comment - Hi Michael, The error on 1.1.x is like: Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 18.453 sec <<< FAILURE! testSetFirstResults(org.apache.openjpa.persistence.jpql.clauses.TestEJBQueryInterface) Time elapsed: 0.359 sec <<< FAILURE! junit.framework.ComparisonFailure: expected:<...3> but was:<...0> at junit.framework.Assert.assertEquals(Assert.java:81) at junit.framework.Assert.assertEquals(Assert.java:87) at org.apache.openjpa.persistence.jpql.clauses.TestEJBQueryInterface.testSetFirstResults(TestEJBQueryInterface.java:124) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at junit.framework.TestCase.runTest(TestCase.java:154) at junit.framework.TestCase.runBare(TestCase.java:127) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118) at org.apache.openjpa.persistence.test.PersistenceTestCase.run(PersistenceTestCase.java:127) at junit.framework.TestSuite.runTest(TestSuite.java:208) at junit.framework.TestSuite.run(TestSuite.java:203) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.maven.surefire.junit.JUnitTestSet.execute(JUnitTestSet.java:213) at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:140) at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:127) at org.apache.maven.surefire.Surefire.run(Surefire.java:177) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:334) at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:980) the insertion order seems random so the error changes every time. I also tried on trunk but can't reproduce the error. Do we maintain the order of the creation of java instances somewhere?
          Hide
          Michael Dick added a comment -

          Hi Amy,

          The test passes as is for me on 1.2.x, and trunk.

          This is with the Java 1.5 (IBM and Sun) on 64 bit Linux (x86 fwiw). Is anyone else able to reproduce this problem?

          Show
          Michael Dick added a comment - Hi Amy, The test passes as is for me on 1.2.x, and trunk. This is with the Java 1.5 (IBM and Sun) on 64 bit Linux (x86 fwiw). Is anyone else able to reproduce this problem?
          Hide
          Amy Yang added a comment -

          Hi Donald, i created the patch on branch 1.1.x

          Show
          Amy Yang added a comment - Hi Donald, i created the patch on branch 1.1.x
          Hide
          Donald Woods added a comment -

          Amy, which branch was used to create the patch - 1.1.x, 1.2.x, trunk, ...?

          Show
          Donald Woods added a comment - Amy, which branch was used to create the patch - 1.1.x, 1.2.x, trunk, ...?
          Hide
          Amy Yang added a comment -

          patch attached.

          Show
          Amy Yang added a comment - patch attached.

            People

            • Assignee:
              Michael Dick
              Reporter:
              Amy Yang
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development