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

ClassCastException occurs when an equals comparison query is executed on an entity with an @EmbeddedId that contains more than one field.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2.3, 2.4.1
    • Fix Version/s: 2.1.2, 2.2.1.1, 2.2.3, 2.4.2
    • Component/s: criteria, query, sql
    • Labels:
      None

      Description

      Take the following entity:

      @Entity
      public class Subject implements Serializable {

      @EmbeddedId
      private SubjectKey key;
      .......

      Where SubjectKey is as follows:

      @Embeddable
      public class SubjectKey implements Serializable {
      private Integer subjectNummer;

      private String subjectTypeCode;
      ......

      As you can see we have a composite primary key. With this, take this query:

      TypedQuery<Subject> query = em.createQuery("select s from Subject s where s = :subject", Subject.class);
      query.setParameter("subject", s);
      Subject s2 = query.getSingleResult();

      This query will yield the following exception:

      java.lang.ClassCastException: org.apache.openjpa.persistence.embed.compositepk.SubjectKey cannot be cast to
      [Ljava.lang.Object;]
      at org.apache.openjpa.jdbc.kernel.exps.Param.appendTo(Param.java:149)

      If we execute a corresponding 'em.find' of Subject, this exception doesn't occur. Furthermore, if you execute the same query for an entity with an @EmbeddedId that only contains one field, all will work as expected. The issue here is with an equals query where the entity contains an @EmbeddableId with more than two fields.

      While investigating/debugging this issue, I've found further issues when creating the query using CriteriaBuilder; both with an @Embeddable and @IdClass composite PKs. I will leave it as an exercise for the reader to view the attached test case to see how each issue can occur. Each test method details what issue it recreated before the fixes to this issue. I'm also attaching a patch with a proposed fix for the issues.

      Thanks,

      Heath Thomann

        Issue Links

          Activity

          Hide
          jpaheath Heath Thomann added a comment -

          For posterity's sake, and given this was a complex issue to debug/fix, I'd like to document some of the steps I took to debug this....sorry, it might be a bit awkward but better than nothing should I/we ever need to revisit this issue. This debug assumes knowledge of the attached test. The issue occurs here:

          public void appendTo(Select sel, ExpContext ctx, ExpState state,
          SQLBuffer sql, int index) {
          ParamExpState pstate = (ParamExpState) state;
          if (pstate.otherLength > 1)
          sql.appendValue(((Object[]) pstate.sqlValue)[index], <------ line 149
          pstate.getColumn(index), this);

          Note that I added a system out in this 'if' block, then I ran the OpenJPA JUnit bucket. I found that ONLY one test method ever hits this 'if' statement.....so it seems this 'if' block is very rarely executed (which I suppose explains why we have yet to see this issue). Anyway, in the debugger I notice that 'pstate.otherLength' was 2, which is the size of the number of columns in my PK (SubjectKey), yet 'pstate.sqlValue' contained 'Subject' (the entity in the attacked test). So I suspected that this code expected to get at the column values of the PK (SubjectKey), not the entity (Subject) itself. So I walked backwards to see where 'otherLength' and 'sqlValue' where set. I found that here in Param:

          public void calculateValue(Select sel, ExpContext ctx, ExpState state,
          Val other, ExpState otherState) {
          super.calculateValue(sel, ctx, state, other, otherState);
          Object val = getValue(ctx.params);
          ParamExpState pstate = (ParamExpState) state;
          if (other != null && !_container) {
          pstate.sqlValue = other.toDataStoreValue(sel, ctx, otherState, val);
          pstate.otherLength = other.length(sel, ctx, otherState);

          other.toDataStoreValue calls to ClassMapping.toDataStoreValue....here is that code (note the javadoc):

          /**

          • Return the given column value(s) for the given object. The given
          • columns will be primary key columns of this mapping, but may be in
          • any order. If there is only one column, return its value. If there
          • are multiple columns, return an object array of their values, in the
          • same order the columns are given.
            */
            public Object toDataStoreValue(Object obj, Column[] cols, JDBCStore store) {
            Object ret = (cols.length == 1) ? null : new Object[cols.length];

          // in the past we've been lenient about being able to translate objects
          // from other persistence contexts, so try to get sm directly from
          // instance before asking our context
          OpenJPAStateManager sm;
          if (ImplHelper.isManageable(obj)) {
          PersistenceCapable pc = ImplHelper.toPersistenceCapable(obj,
          getRepository().getConfiguration());
          sm = (OpenJPAStateManager) pc.pcGetStateManager();
          if (sm == null) {
          ret = getValueFromUnmanagedInstance(obj, cols, true);

          In my scenario 'sm' is null so we take the block to 'ret = getValueFromUnmanagedInstance'. Again, note that I added a system out in this 'if' block, then I ran the OpenJPA test bucket and only one test method ever hits this 'if' statement (again, nearly dead code). When I ran my test, I notice that 'ret' is assigned 'Subject' after a call to 'getValueFromUnmanagedInstance, not the PK values as promised by the javadoc listed above. So I set off to figure out how to get the PKs columns from Subject, and their values. To do this, I thought "if we execute this query as a 'find' instead, how does that path extract the PK columns and values." When running in a debugger, I saw that we go into this code in SelectImpl.where:

          join = mapping.assertJoinable(toCols[i]);
          val = pks[mapping.getField(join.getFieldIndex()).
          getPrimaryKeyIndex()];
          val = join.getJoinValue(val, toCols[i], store);

          For a finder, this is where we get the PK of SubjectKey and get its individual values of the SubjectKey.....this is where I took my idea for the fix attached to this JIRA.

          Thanks,

          Heath

          Show
          jpaheath Heath Thomann added a comment - For posterity's sake, and given this was a complex issue to debug/fix, I'd like to document some of the steps I took to debug this....sorry, it might be a bit awkward but better than nothing should I/we ever need to revisit this issue. This debug assumes knowledge of the attached test. The issue occurs here: public void appendTo(Select sel, ExpContext ctx, ExpState state, SQLBuffer sql, int index) { ParamExpState pstate = (ParamExpState) state; if (pstate.otherLength > 1) sql.appendValue(((Object[]) pstate.sqlValue) [index] , <------ line 149 pstate.getColumn(index), this); Note that I added a system out in this 'if' block, then I ran the OpenJPA JUnit bucket. I found that ONLY one test method ever hits this 'if' statement.....so it seems this 'if' block is very rarely executed (which I suppose explains why we have yet to see this issue). Anyway, in the debugger I notice that 'pstate.otherLength' was 2, which is the size of the number of columns in my PK (SubjectKey), yet 'pstate.sqlValue' contained 'Subject' (the entity in the attacked test). So I suspected that this code expected to get at the column values of the PK (SubjectKey), not the entity (Subject) itself. So I walked backwards to see where 'otherLength' and 'sqlValue' where set. I found that here in Param: public void calculateValue(Select sel, ExpContext ctx, ExpState state, Val other, ExpState otherState) { super.calculateValue(sel, ctx, state, other, otherState); Object val = getValue(ctx.params); ParamExpState pstate = (ParamExpState) state; if (other != null && !_container) { pstate.sqlValue = other.toDataStoreValue(sel, ctx, otherState, val); pstate.otherLength = other.length(sel, ctx, otherState); other.toDataStoreValue calls to ClassMapping.toDataStoreValue....here is that code (note the javadoc): /** Return the given column value(s) for the given object. The given columns will be primary key columns of this mapping, but may be in any order. If there is only one column, return its value. If there are multiple columns, return an object array of their values, in the same order the columns are given. */ public Object toDataStoreValue(Object obj, Column[] cols, JDBCStore store) { Object ret = (cols.length == 1) ? null : new Object [cols.length] ; // in the past we've been lenient about being able to translate objects // from other persistence contexts, so try to get sm directly from // instance before asking our context OpenJPAStateManager sm; if (ImplHelper.isManageable(obj)) { PersistenceCapable pc = ImplHelper.toPersistenceCapable(obj, getRepository().getConfiguration()); sm = (OpenJPAStateManager) pc.pcGetStateManager(); if (sm == null) { ret = getValueFromUnmanagedInstance(obj, cols, true); In my scenario 'sm' is null so we take the block to 'ret = getValueFromUnmanagedInstance'. Again, note that I added a system out in this 'if' block, then I ran the OpenJPA test bucket and only one test method ever hits this 'if' statement (again, nearly dead code). When I ran my test, I notice that 'ret' is assigned 'Subject' after a call to 'getValueFromUnmanagedInstance, not the PK values as promised by the javadoc listed above. So I set off to figure out how to get the PKs columns from Subject, and their values. To do this, I thought "if we execute this query as a 'find' instead, how does that path extract the PK columns and values." When running in a debugger, I saw that we go into this code in SelectImpl.where: join = mapping.assertJoinable(toCols [i] ); val = pks[mapping.getField(join.getFieldIndex()). getPrimaryKeyIndex()]; val = join.getJoinValue(val, toCols [i] , store); For a finder, this is where we get the PK of SubjectKey and get its individual values of the SubjectKey.....this is where I took my idea for the fix attached to this JIRA. Thanks, Heath
          Hide
          jpaheath Heath Thomann added a comment -

          I'm attaching a patch which contains the proposed final fix and tests.

          Thanks,

          Heath

          Show
          jpaheath Heath Thomann added a comment - I'm attaching a patch which contains the proposed final fix and tests. Thanks, Heath
          Hide
          ilgrosso Francesco Chicchiriccò added a comment -

          Honestly, both the problem and the fix were not easy to understand, but I think I finally managed to do it

          AFAICT, the patch looks very good and solves a very nasty problem.

          I have also applied your patch to trunk and verified that all tests are passing.

          One question, though: I see this commented block in TestCompositePrimaryKeys:

          // This works:
          // Predicate subjectPredicate1 = builder.equal(subjectRoot.get(Subject_.key).get(SubjectKey_.subjectNummer),
          // subject.getKey().getSubjectNummer());
          // Predicate subjectPredicate2 = builder.equal(subjectRoot.get(Subject_.key).get(SubjectKey_.subjectTypeCode),
          // subject.getKey().getSubjectTypeCode());
          // Predicate subjectPredicate = builder.and(subjectPredicate1,subjectPredicate2);

          Can you explain why you have left it there? Or, alternatively, why haven't you uncommented it?

          Finally, minor stuff which can be easily adjusted:

          2013 test WARN [main] openjpa.MetaData - The composite identity class "class org.apache.openjpa.persistence.embed.compositepk.SubjectIdClass" for entity "class org.apache.openjpa.persistence.embed.compositepk.SubjectWithIdClass" is not serializable.
          2520 test WARN [main] openjpa.MetaData - The composite identity class "class org.apache.openjpa.persistence.embed.compositepk.SubjectIdClass" for entity "class org.apache.openjpa.persistence.embed.compositepk.SubjectWithIdClass" is not serializable.

          Thanks!

          Show
          ilgrosso Francesco Chicchiriccò added a comment - Honestly, both the problem and the fix were not easy to understand, but I think I finally managed to do it AFAICT, the patch looks very good and solves a very nasty problem. I have also applied your patch to trunk and verified that all tests are passing. One question, though: I see this commented block in TestCompositePrimaryKeys: // This works: // Predicate subjectPredicate1 = builder.equal(subjectRoot.get(Subject_.key).get(SubjectKey_.subjectNummer), // subject.getKey().getSubjectNummer()); // Predicate subjectPredicate2 = builder.equal(subjectRoot.get(Subject_.key).get(SubjectKey_.subjectTypeCode), // subject.getKey().getSubjectTypeCode()); // Predicate subjectPredicate = builder.and(subjectPredicate1,subjectPredicate2); Can you explain why you have left it there? Or, alternatively, why haven't you uncommented it? Finally, minor stuff which can be easily adjusted: 2013 test WARN [main] openjpa.MetaData - The composite identity class "class org.apache.openjpa.persistence.embed.compositepk.SubjectIdClass" for entity "class org.apache.openjpa.persistence.embed.compositepk.SubjectWithIdClass" is not serializable. 2520 test WARN [main] openjpa.MetaData - The composite identity class "class org.apache.openjpa.persistence.embed.compositepk.SubjectIdClass" for entity "class org.apache.openjpa.persistence.embed.compositepk.SubjectWithIdClass" is not serializable. Thanks!
          Hide
          jpaheath Heath Thomann added a comment -

          Thanks for the review and comments Francesco! Regarding that commented out code, it was deliberately commented out. Prior to this fix, it was a work around. In other words, if you created a Criteria which selected the individual fields of the PK then the test would work. For history sake I left that commented code in the test case, but given your confusing I've added better comments about why I left it in there.
          Finally, I made SubjectIdClass Serializable. Thanks again for the detailed review!

          Thanks,

          Heath

          Show
          jpaheath Heath Thomann added a comment - Thanks for the review and comments Francesco! Regarding that commented out code, it was deliberately commented out. Prior to this fix, it was a work around. In other words, if you created a Criteria which selected the individual fields of the PK then the test would work. For history sake I left that commented code in the test case, but given your confusing I've added better comments about why I left it in there. Finally, I made SubjectIdClass Serializable. Thanks again for the detailed review! Thanks, Heath
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1750036 from Heath Thomann in branch 'openjpa/branches/2.1.x'
          [ https://svn.apache.org/r1750036 ]

          OPENJPA-2631: Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1750036 from Heath Thomann in branch 'openjpa/branches/2.1.x' [ https://svn.apache.org/r1750036 ] OPENJPA-2631 : Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field.
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          OPENJPA-2631: Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to 2.2.x

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1750037 from Heath Thomann in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1750037 ] OPENJPA-2631 : Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to 2.2.x
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          OPENJPA-2631: Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to trunk

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1750038 from Heath Thomann in branch 'openjpa/trunk' [ https://svn.apache.org/r1750038 ] OPENJPA-2631 : Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to trunk
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1750039 from Heath Thomann in branch 'openjpa/branches/2.2.1.x'
          [ https://svn.apache.org/r1750039 ]

          OPENJPA-2631: Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to 2.2.1

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1750039 from Heath Thomann in branch 'openjpa/branches/2.2.1.x' [ https://svn.apache.org/r1750039 ] OPENJPA-2631 : Fix for CriteriaBuilder issue with an @EmbeddedId that contains more than one field. Ported 2.1.x commit to 2.2.1
          Hide
          ilgrosso Francesco Chicchiriccò added a comment -

          You're welcome! Glad that you applied the patch to all recent branches, thanks.

          Show
          ilgrosso Francesco Chicchiriccò added a comment - You're welcome! Glad that you applied the patch to all recent branches, thanks.
          Hide
          ilgrosso Francesco Chicchiriccò added a comment -

          Bulk close for 2.4.2

          Show
          ilgrosso Francesco Chicchiriccò added a comment - Bulk close for 2.4.2
          Hide
          dazeydev Will Dazey added a comment -

          I believe this fix introduced a failure. org.apache.openjpa.jdbc.meta.ClassMapping throws an ArrayOutOfBoundsException if using an @EmbeddedId. getPrimaryKeyFieldMappings() on the Entity with the EmbeddedId will return an array with length = 0. A length check needs to be added.

          Show
          dazeydev Will Dazey added a comment - I believe this fix introduced a failure. org.apache.openjpa.jdbc.meta.ClassMapping throws an ArrayOutOfBoundsException if using an @EmbeddedId. getPrimaryKeyFieldMappings() on the Entity with the EmbeddedId will return an array with length = 0. A length check needs to be added.
          Hide
          dazeydev Will Dazey added a comment -
          Show
          dazeydev Will Dazey added a comment - Opened https://issues.apache.org/jira/browse/OPENJPA-2705 to fix this issue

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development