OpenJPA
  1. OpenJPA
  2. OPENJPA-904 Testcase failures when using the PostgreSQL, Oracle, and DB2 databases
  3. OPENJPA-1090

Oracle failures due to the following warning "This database dictionary "Oracle" does not support auto-assigned column values. The column "pid" may not behave as desired."

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-M2
    • Component/s: None
    • Labels:
      None
    • Patch Info:
      Patch Available
    1. OPENJPA-1090_3.patch
      19 kB
      Tim McConnell
    2. OPENJPA-1090_2.patch
      52 kB
      Tim McConnell

      Activity

      Tim McConnell created issue -
      Tim McConnell made changes -
      Field Original Value New Value
      Status Open [ 1 ] In Progress [ 3 ]
      Hide
      Tim McConnell added a comment -

      This patch will check to ensure that the database supports auto-assign column(s) before attempting to use Entities with this annotation::

      @GeneratedValue(strategy=GenerationType.IDENTITY)

      Show
      Tim McConnell added a comment - This patch will check to ensure that the database supports auto-assign column(s) before attempting to use Entities with this annotation:: @GeneratedValue(strategy=GenerationType.IDENTITY)
      Tim McConnell made changes -
      Attachment OPENJPA-1090.patch [ 12408305 ]
      Donald Woods made changes -
      Patch Info [Patch Available]
      Hide
      Donald Woods added a comment -

      Tim, can you use the existing DBDictionary.supportsAutoAssign boolean instead of the try/catch block in setup(), or is it not getting set correctly for Oracle?

      Show
      Donald Woods added a comment - Tim, can you use the existing DBDictionary.supportsAutoAssign boolean instead of the try/catch block in setup(), or is it not getting set correctly for Oracle?
      Hide
      Tim McConnell added a comment -

      Thanks much for reviewing Donald. I'll investigate and change accordingly.....

      Show
      Tim McConnell added a comment - Thanks much for reviewing Donald. I'll investigate and change accordingly.....
      Hide
      Tim McConnell added a comment -

      Second patch has been attached after incorporating Donald's comments.....

      Show
      Tim McConnell added a comment - Second patch has been attached after incorporating Donald's comments.....
      Tim McConnell made changes -
      Attachment OPENJPA-1090_2.patch [ 12408869 ]
      Tim McConnell made changes -
      Attachment OPENJPA-1090.patch [ 12408305 ]
      Hide
      Milosz Tylenda added a comment -

      Hi Tim & Donald. If I can have a remark I would suggest that the code will be more readable and the patch much smaller if instead of:

      testMethod() {
      if (supportsAutoAssign)

      { [method body] }

      }

      we do:

      testMethod() {
      if (!supportsAutoAssign)

      { return; }

      [method body]
      }

      This is how, for example, TestGenerationType does it (which, BTW, does not need to be modified by the patch)

      Show
      Milosz Tylenda added a comment - Hi Tim & Donald. If I can have a remark I would suggest that the code will be more readable and the patch much smaller if instead of: testMethod() { if (supportsAutoAssign) { [method body] } } we do: testMethod() { if (!supportsAutoAssign) { return; } [method body] } This is how, for example, TestGenerationType does it (which, BTW, does not need to be modified by the patch)
      Hide
      Tim McConnell added a comment -

      Hi Milosz, Not sure I agree or not since I tend to prefer single return points from methods (when possible), but I do see and appreciate your point. So, what I'll do is add another patch doing as you suggest and let whomever commits the patch be the final arbiter

      Thanks much for reviewing.....

      Show
      Tim McConnell added a comment - Hi Milosz, Not sure I agree or not since I tend to prefer single return points from methods (when possible), but I do see and appreciate your point. So, what I'll do is add another patch doing as you suggest and let whomever commits the patch be the final arbiter Thanks much for reviewing.....
      Hide
      Tim McConnell added a comment -

      Attaching smaller patch incorporating Milosz's comments.....

      Show
      Tim McConnell added a comment - Attaching smaller patch incorporating Milosz's comments.....
      Tim McConnell made changes -
      Attachment OPENJPA-1090_3.patch [ 12408888 ]
      Tim McConnell made changes -
      Status In Progress [ 3 ] Resolved [ 5 ]
      Fix Version/s 2.0.0 [ 12313483 ]
      Resolution Fixed [ 1 ]
      Donald Woods made changes -
      Status Resolved [ 5 ] Closed [ 6 ]

        People

        • Assignee:
          Tim McConnell
          Reporter:
          Tim McConnell
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development