Uploaded image for project: 'OpenJPA'
  1. OpenJPA
  2. OPENJPA-2651

IDs of entities are incorrectly assigned when @SqlResultSetMapping is used with inheritance and a ManyToOne relationship.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2.2, 2.3.0, 2.4.1
    • Fix Version/s: 2.2.3, 2.4.0, 2.4.2
    • Component/s: sql
    • Labels:
      None

      Description

      I have discovered an issue with using @SqlResultSetMapping when inheritance and ManyToOne relationships are used. To explain the issue, take this entity and @SqlResultSetMapping:

      @Entity
      @SqlResultSetMapping(name = "MyResMap", entities = { @EntityResult(entityClass = CrtRequisicaoChequePersEntity.class, fields =

      { @FieldResult(name = "crtOperacaoByOperacaoRecepcaoServCent.id", column = "opRecepcaoServCentraisId"), @FieldResult(name = "crtOperacaoByOperacaoRecepcaoServCent.dataHora", column = "opRecepcaoServCentraisDataHora") }

      )
      })
      public class CrtRequisicaoChequePersEntity extends CrtRequisicaoEntity {
      .......
      @ManyToOne
      @javax.persistence.JoinColumn(name = "OPERACAO_RECEPCAO_SERV_CENT", referencedColumnName = "ID")
      private CrtOperacaoEntity crtOperacaoByOperacaoRecepcaoServCent;

      As you can see, this entity extends 'CrtRequisicaoEntity' and has a ManyToOne relationship to 'CrtOperacaoEntity', with name 'crtOperacaoByOperacaoRecepcaoServCent'. As you can see, the @FieldResult in the @SqlResultSetMapping references the fields in 'CrtOperacaoEntity'. These two entities are defined as follows:

      @Entity
      @Inheritance(strategy = InheritanceType.JOINED)
      public class CrtRequisicaoEntity {
      .....
      @Id
      private long id;
      .....

      @Entity
      public class CrtOperacaoEntity implements Serializable {
      .....
      @Id
      private long id;

      @Basic
      private Timestamp dataHora;

      With these entities, take an SQL select which uses an AS (the entire SQL is to long to add here, see provided recreate/test attached):

      String sqlCust = "SELECT t0.ID" +
      .........
      ",t2.DATA_HORA as opRecepcaoServCentraisDataHora" +
      ",t2.ID as opRecepcaoServCentraisId" +
      ....
      "FROM CrtRequisicaoChequePersEntity t0"
      ....
      "INNER JOIN CrtOperacaoEntity t2"
      ....

      With this SQL, the two IDs will be populated with the ID from CrtRequisicaoChequePersEntity, rather than the ID corresponding to each entity.

      Thanks,

      Heath

      1. OPENJPA-2651-2.2.x.patch
        15 kB
        Heath Thomann
      2. SqlResultSetMappingIssue.zip
        17 kB
        Heath Thomann

        Activity

        Hide
        jpaheath Heath Thomann added a comment -

        I am attaching a crude/unpolished test which can be used to recreate the reported issue. The entities involved and SQL involved are rather large. As such I am not able to boil down this test into something suitable for a JUnit.....maybe some day.....
        To run the test you can edit the .classpath to suit your environment. The .zip contains a file named create.sql to set up the database (this is for DB2). You can then import the .zip into Eclipse, or your favorite IDE, and run the test named MainStocks.

        Thanks,

        Heath

        Show
        jpaheath Heath Thomann added a comment - I am attaching a crude/unpolished test which can be used to recreate the reported issue. The entities involved and SQL involved are rather large. As such I am not able to boil down this test into something suitable for a JUnit.....maybe some day..... To run the test you can edit the .classpath to suit your environment. The .zip contains a file named create.sql to set up the database (this is for DB2). You can then import the .zip into Eclipse, or your favorite IDE, and run the test named MainStocks. Thanks, Heath
        Hide
        jpaheath Heath Thomann added a comment -

        Attaching proposed fix.

        Thanks,

        Heath Thomann

        Show
        jpaheath Heath Thomann added a comment - Attaching proposed fix. Thanks, Heath Thomann
        Hide
        ilgrosso Francesco Chicchiriccò added a comment -

        LGTM: are you keen to provide a test case too?

        Show
        ilgrosso Francesco Chicchiriccò added a comment - LGTM: are you keen to provide a test case too?
        Hide
        jpaheath Heath Thomann added a comment -

        OK, Francesco Chicchiriccò request for a test prompted me to take the time to boil down the test I previous provided in the .zip to a JUnit test which could be committed. I'm attaching a full patch, named OPENJPA-2651-2.2.x.patch, with test to recreate the issue and fix.

        Thanks,

        Heath

        Show
        jpaheath Heath Thomann added a comment - OK, Francesco Chicchiriccò request for a test prompted me to take the time to boil down the test I previous provided in the .zip to a JUnit test which could be committed. I'm attaching a full patch, named OPENJPA-2651 -2.2.x.patch, with test to recreate the issue and fix. Thanks, Heath
        Hide
        ilgrosso Francesco Chicchiriccò added a comment -

        Thanks for this: it's always a good chance for me to get a bit more acquainted with the (huge) OpenJPA codebase

        Anyway, the patch looks good, I've applied locally and been able to build OpenJPA 2.2.x with it.

        Are you also going to provide patches for 2.3.x and trunk as well?

        Finally, why is not the "affected versions" above set to 2.2.2, 2.3.0 and 2.4.1? I would also set "fix for versions" to 2.2.3, 2.3.1 and 2.4.2.

        Thanks again.

        Show
        ilgrosso Francesco Chicchiriccò added a comment - Thanks for this: it's always a good chance for me to get a bit more acquainted with the (huge) OpenJPA codebase Anyway, the patch looks good, I've applied locally and been able to build OpenJPA 2.2.x with it. Are you also going to provide patches for 2.3.x and trunk as well? Finally, why is not the "affected versions" above set to 2.2.2, 2.3.0 and 2.4.1? I would also set "fix for versions" to 2.2.3, 2.3.1 and 2.4.2. Thanks again.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1756046 from Heath Thomann in branch 'openjpa/branches/2.2.x'
        [ https://svn.apache.org/r1756046 ]

        OPENJPA-2651: Fix for issue where @SqlResultSetMapping is used with inheritance and a ManyToOne relationship.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1756046 from Heath Thomann in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1756046 ] OPENJPA-2651 : Fix for issue where @SqlResultSetMapping is used with inheritance and a ManyToOne relationship.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1756047 from Heath Thomann in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1756047 ]

        OPENJPA-2651: Fix for issue where @SqlResultSetMapping is used with inheritance and a ManyToOne relationship. Merged 2.2.x commit to trunk.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1756047 from Heath Thomann in branch 'openjpa/trunk' [ https://svn.apache.org/r1756047 ] OPENJPA-2651 : Fix for issue where @SqlResultSetMapping is used with inheritance and a ManyToOne relationship. Merged 2.2.x commit to trunk.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1756081 from Francesco Chicchiriccò in branch 'openjpa/branches/2.3.x'
        [ https://svn.apache.org/r1756081 ]

        OPENJPA-2651 Applying Heath Thomann's patch for 2.2.x to 2.3.x

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1756081 from Francesco Chicchiriccò in branch 'openjpa/branches/2.3.x' [ https://svn.apache.org/r1756081 ] OPENJPA-2651 Applying Heath Thomann's patch for 2.2.x to 2.3.x
        Hide
        ilgrosso Francesco Chicchiriccò added a comment -

        I went ahead and applied your patch to 2.3.x, after verifying it was working fine.
        Shall we resolve this issue, then?

        I also created https://builds.apache.org/view/OpenJPA/job/OpenJPA-23x which appeared to be missing.

        Regards.

        Show
        ilgrosso Francesco Chicchiriccò added a comment - I went ahead and applied your patch to 2.3.x, after verifying it was working fine. Shall we resolve this issue, then? I also created https://builds.apache.org/view/OpenJPA/job/OpenJPA-23x which appeared to be missing. Regards.
        Hide
        jpaheath Heath Thomann added a comment -

        Hi Francesco! Let me answer your questions:

        Q1) Are you also going to provide patches for 2.3.x and trunk as well?

        A1) I applied to 2.2.x and trunk. I don't use 2.3.x and was thinking others did so I let the interested parties put stuff into 2.3.x as they see fit. I don't know, maybe no one is using 2.3.x these days??

        Q2) Finally, why is not the "affected versions" above set to 2.2.2, 2.3.0 and 2.4.1? I would also set "fix for versions" to 2.2.3, 2.3.1 and 2.4.2.

        A2) Your question is an interesting one......one could spend all day updating "affected versions". Personally, I use the versions I test on, which is always the latest version of a particular branch. For example, the testing I was doing was on 2.2.x, and therefore I was using the latest code, which is 2.2.3. If I was on 1.2.x I'd be using 1.2.4, and as such list it. Continuing with 1.2.4, I'd take the testing and fix forward to 2.0.x, 2.1.x, 2.2.x, and trunk; where applicable. As such, I'd list those as the affected versions.

        Q3) Shall we resolve this issue, then?

        A3) Yes, was getting around to it.

        I hope this helps.

        Thanks,

        Heath

        Show
        jpaheath Heath Thomann added a comment - Hi Francesco! Let me answer your questions: Q1) Are you also going to provide patches for 2.3.x and trunk as well? A1) I applied to 2.2.x and trunk. I don't use 2.3.x and was thinking others did so I let the interested parties put stuff into 2.3.x as they see fit. I don't know, maybe no one is using 2.3.x these days?? Q2) Finally, why is not the "affected versions" above set to 2.2.2, 2.3.0 and 2.4.1? I would also set "fix for versions" to 2.2.3, 2.3.1 and 2.4.2. A2) Your question is an interesting one......one could spend all day updating "affected versions". Personally, I use the versions I test on, which is always the latest version of a particular branch. For example, the testing I was doing was on 2.2.x, and therefore I was using the latest code, which is 2.2.3. If I was on 1.2.x I'd be using 1.2.4, and as such list it. Continuing with 1.2.4, I'd take the testing and fix forward to 2.0.x, 2.1.x, 2.2.x, and trunk; where applicable. As such, I'd list those as the affected versions. Q3) Shall we resolve this issue, then? A3) Yes, was getting around to it. I hope this helps. Thanks, Heath
        Hide
        ilgrosso Francesco Chicchiriccò added a comment -

        I don't know, maybe no one is using 2.3.x these days?

        The Syncope maintenance branch 1_2_X [1] is doing that, here's why I've gladly applied your patch

        For example, the testing I was doing was on 2.2.x, and therefore I was using the latest code, which is 2.2.3. If I was on 1.2.x I'd be using 1.2.4, and as such list it. Continuing with 1.2.4, I'd take the testing and fix forward to 2.0.x, 2.1.x, 2.2.x, and trunk; where applicable. As such, I'd list those as the affected versions.

        You've been actually testing on (and providing patches for) 2.2.3-SNAPSHOT and 2.4.2-SNAPSHOT, hence the fix will be delivered when we'll release 2.2.3 and 2.4.2: here's why the fix-for-version is important to correctly include 2.2.3 and 2.4.2, to get through to appropriate release notes.

        Thanks again for your work.

        [1] https://git-wip-us.apache.org/repos/asf?p=syncope.git;a=shortlog;h=refs/heads/1_2_X

        Show
        ilgrosso Francesco Chicchiriccò added a comment - I don't know, maybe no one is using 2.3.x these days? The Syncope maintenance branch 1_2_X [1] is doing that, here's why I've gladly applied your patch For example, the testing I was doing was on 2.2.x, and therefore I was using the latest code, which is 2.2.3. If I was on 1.2.x I'd be using 1.2.4, and as such list it. Continuing with 1.2.4, I'd take the testing and fix forward to 2.0.x, 2.1.x, 2.2.x, and trunk; where applicable. As such, I'd list those as the affected versions. You've been actually testing on (and providing patches for) 2.2.3-SNAPSHOT and 2.4.2-SNAPSHOT, hence the fix will be delivered when we'll release 2.2.3 and 2.4.2: here's why the fix-for-version is important to correctly include 2.2.3 and 2.4.2, to get through to appropriate release notes. Thanks again for your work. [1] https://git-wip-us.apache.org/repos/asf?p=syncope.git;a=shortlog;h=refs/heads/1_2_X

          People

          • Assignee:
            jpaheath Heath Thomann
            Reporter:
            jpaheath Heath Thomann
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development