OpenJPA
  1. OpenJPA
  2. OPENJPA-331

Allow BigInteger and other Basic types as Primary Keys

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.7, 1.0.0
    • Fix Version/s: 1.0.2, 1.1.0
    • Component/s: kernel
    • Labels:
      None

      Description

      Section 2.1.4 of the JPA spec outlines the requirements for a primary key (@Id):

      Section 2.1.4: A simple ( i.e., non-composite) primary key must correspond to a single persistent field or property of
      the entity class. The Id annotation is used to denote a simple primary key. See section 9.1.8.

      Also from Section 2.1.4 : The primary key (or field or property of a composite primary key) should be one of the following types:
      any Java primitive type; any primitive wrapper type; java.lang.String; java.util.Date;
      java.sql.Date. In general, however, approximate numeric types (e.g., floating point types) should
      never be used in primary keys. Entities whose primary keys use types other than these will not be portable.
      If generated primary keys are used, only integral types will be portable. If java.util.Date is
      used as a primary key field or property, the temporal type should be specified as DATE.

      Right now, we are treating the "should be" statement above as "must be" and only allowing the specified types as primary keys. But, the sentence in the middle of this paragraph ("Entities whose primary keys use types other than these will not be portable.") indicates that other types can also be allowed, but may not be portable.

      It seems that Glassfish allows these additional Basic types as primary keys. We should probably allow the same thing to keep up with the Jones'.

      This was discussed on dev mailing list. http://www.nabble.com/BigInteger-as-%40Id-tf4318071.html

      1. fix-OpenJPA-331.patch
        19 kB
        Miroslav Nachev
      2. license-fix-OpenJPA-331-v110.patch
        12 kB
        Miroslav Nachev
      3. license-fix-OpenJPA-331-v110.patch
        12 kB
        Miroslav Nachev
      4. license-fix-OpenJPA-331-v110.patch
        12 kB
        Miroslav Nachev

        Activity

        Hide
        Albert Lee added a comment -

        Defer to next release.

        Show
        Albert Lee added a comment - Defer to next release.
        Hide
        Patrick Linskey added a comment -

        Resolved in trunk; keeping open since this probably deserves to be in 1.0.2, but I cannot apply patches to 1.0.2 on this machine due to a lame patch implementation.

        The new files have a slightly different ASL license banner at the top. To the best of my non-legally-schooled knowledge, it looks right, and I'm reluctant to change someone else's license banner, but it seemed worth highlighting.

        Show
        Patrick Linskey added a comment - Resolved in trunk; keeping open since this probably deserves to be in 1.0.2, but I cannot apply patches to 1.0.2 on this machine due to a lame patch implementation. The new files have a slightly different ASL license banner at the top. To the best of my non-legally-schooled knowledge, it looks right, and I'm reluctant to change someone else's license banner, but it seemed worth highlighting.
        Hide
        Patrick Linskey added a comment -

        Oh, and thanks for the patch, Miro!

        One more note: it looks like the automated build machine is currently offline, so this won't make it into a snapshot until it comes back online or someone manually creates a snapshot. I will not be able to debug the offline machine until late next week.

        Show
        Patrick Linskey added a comment - Oh, and thanks for the patch, Miro! One more note: it looks like the automated build machine is currently offline, so this won't make it into a snapshot until it comes back online or someone manually creates a snapshot. I will not be able to debug the offline machine until late next week.
        Hide
        Craig L Russell added a comment -

        Hi Miro,

        Thanks for the patch! The copyright and license in the headers of the new files are an issue that I'd like you to fix and resubmit. Since the changes have already been checked in, an update to the headers and/or NOTICE.txt is all that's needed.

        Please see http://cwiki.apache.org/confluence/display/openjpa/Found+a+Bug for details on the headers that are needed.

        Thanks,

        Craig

        Show
        Craig L Russell added a comment - Hi Miro, Thanks for the patch! The copyright and license in the headers of the new files are an issue that I'd like you to fix and resubmit. Since the changes have already been checked in, an update to the headers and/or NOTICE.txt is all that's needed. Please see http://cwiki.apache.org/confluence/display/openjpa/Found+a+Bug for details on the headers that are needed. Thanks, Craig
        Hide
        Miroslav Nachev added a comment -

        Is that OK?
        I am so sorry but I am not so friendly with License matter. If this is not OK, please fix it or exact tell me what to do.

        Show
        Miroslav Nachev added a comment - Is that OK? I am so sorry but I am not so friendly with License matter. If this is not OK, please fix it or exact tell me what to do.
        Hide
        Miroslav Nachev added a comment -

        Is that OK?
        I am so sorry but I am not so friendly with License matter. If this is not OK, please fix it or exact tell me what to do.

        Show
        Miroslav Nachev added a comment - Is that OK? I am so sorry but I am not so friendly with License matter. If this is not OK, please fix it or exact tell me what to do.
        Hide
        Craig L Russell added a comment -

        Hi Miro,

        Thanks, this is very close to what we need.

        The license header is ok, but the copyright needs to be removed from the individual files and put into the openjpa-project/NOTICE.txt file. Something like this:

        • OpenJPA includes the following software copyright 2007 Miroslav Nachev

        openjpa-kernel/src/main/java/org/apache/openjpa/util/BigDecimalId.java
        openjpa-kernel/src/main/java/org/apache/openjpa/util/BigIntegerId.java
        openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/SQLBigIntegerIdEntity.java
        openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/SQLBigDecimalIdEntity.java
        openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/TestSQLBigIntegerId.java
        openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/TestSQLBigDecimalId.java

        Thanks,

        Craig

        Show
        Craig L Russell added a comment - Hi Miro, Thanks, this is very close to what we need. The license header is ok, but the copyright needs to be removed from the individual files and put into the openjpa-project/NOTICE.txt file. Something like this: OpenJPA includes the following software copyright 2007 Miroslav Nachev openjpa-kernel/src/main/java/org/apache/openjpa/util/BigDecimalId.java openjpa-kernel/src/main/java/org/apache/openjpa/util/BigIntegerId.java openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/SQLBigIntegerIdEntity.java openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/SQLBigDecimalIdEntity.java openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/TestSQLBigIntegerId.java openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/TestSQLBigDecimalId.java Thanks, Craig
        Hide
        Miroslav Nachev added a comment -

        I hope that now everything is OK.

        Show
        Miroslav Nachev added a comment - I hope that now everything is OK.
        Hide
        Craig L Russell added a comment -

        I've checked in the updated license information into trunk.

        I'll leave the issue open in case someone wants to apply the patch to the branch. I've got some ^M line ending issues when applying the patch to the branch.

        Show
        Craig L Russell added a comment - I've checked in the updated license information into trunk. I'll leave the issue open in case someone wants to apply the patch to the branch. I've got some ^M line ending issues when applying the patch to the branch.
        Hide
        Patrick Linskey added a comment -

        Merged to 1.0.x line.

        Show
        Patrick Linskey added a comment - Merged to 1.0.x line.

          People

          • Assignee:
            Kevin Sutter
            Reporter:
            Kevin Sutter
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development