OpenJPA
  1. OpenJPA
  2. OPENJPA-2018

Cannot bind String[] to ParameterExpression for path.in(parameter)

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.0
    • Fix Version/s: 2.3.0
    • Component/s: jpa

      Description

      Given the following code:

      User user = new User("Dave", "Matthews", "foo@bar.de");
      em.persist(user);
      em.flush();
      
      CriteriaBuilder builder = em.getCriteriaBuilder();
      
      CriteriaQuery<User> criteria = builder.createQuery(User.class);
      Root<User> root = criteria.from(User.class);
      criteria.where(root.get("firstname").in(builder.parameter(String[].class)));
      
      TypedQuery<User> query = em.createQuery(criteria);
      for (ParameterExpression parameter : criteria.getParameters()) {
        query.setParameter(parameter, new String[] {"Dave", "Carter"});
      }
      
      List<User> result = query.getResultList();
      assertThat(result.isEmpty(), is(false));
      

      I get a

      <openjpa-2.0.0-r422266:935683 nonfatal user error> org.apache.openjpa.persistence.ArgumentException: The specified parameter of type "class [Ljava.lang.String;" is not a valid query parameter.
      

      Using Collection as ParameterExpression type and binding the parameters via Arrays.asList(...) works fine.

      1. missingpatch.patch
        7 kB
        Romain Manni-Bucau
      2. OPENJPA-2018.patch
        15 kB
        Romain Manni-Bucau
      3. openjpa-2018.zip
        6 kB
        Oliver Gierke
      4. OPENJPA-2018-test2.patch
        4 kB
        Mark Struberg
      5. OPENJPA-2018-test-update.patch
        6 kB
        Romain Manni-Bucau
      6. OPENJPA-2018-with-array.patch
        21 kB
        Romain Manni-Bucau

        Activity

        Hide
        Oliver Gierke added a comment -

        This still seems to be broken for in 2.3.0 in case you refer to the parameter by name. This works:

        CriteriaBuilder builder = em.getCriteriaBuilder();
        CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class);
        Root<User> root = criteriaQuery.from(User.class);
        ParameterExpression<Collection> parameter = builder.parameter(Collection.class);
        criteriaQuery.where(root.<Integer> get("id").in(parameter));
        TypedQuery<User> query = em.createQuery(criteriaQuery);
        query.setParameter(parameter, Arrays.asList(1, 2));
        List<User> resultList = query.getResultList();

        this doesn't:

        CriteriaBuilder builder = em.getCriteriaBuilder();
        CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class);
        Root<User> root = criteriaQuery.from(User.class);
        criteriaQuery.where(root.<Integer> get("id").in(builder.parameter(Collection.class, "ids")));
        TypedQuery<User> query = em.createQuery(criteriaQuery);
        query.setParameter("ids", Arrays.asList(1, 2));
        List<User> resultList = query.getResultList();

        The latter again fails with the exception:

        Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.List. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration.; nested exception is <openjpa-2.3.0-r422266:1540826 nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.Collection. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration.

        Show
        Oliver Gierke added a comment - This still seems to be broken for in 2.3.0 in case you refer to the parameter by name. This works: CriteriaBuilder builder = em.getCriteriaBuilder(); CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class); Root<User> root = criteriaQuery.from(User.class); ParameterExpression<Collection> parameter = builder.parameter(Collection.class); criteriaQuery.where(root.<Integer> get("id").in(parameter)); TypedQuery<User> query = em.createQuery(criteriaQuery); query.setParameter(parameter, Arrays.asList(1, 2)); List<User> resultList = query.getResultList(); this doesn't: CriteriaBuilder builder = em.getCriteriaBuilder(); CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class); Root<User> root = criteriaQuery.from(User.class); criteriaQuery.where(root.<Integer> get("id").in(builder.parameter(Collection.class, "ids"))); TypedQuery<User> query = em.createQuery(criteriaQuery); query.setParameter("ids", Arrays.asList(1, 2)); List<User> resultList = query.getResultList(); The latter again fails with the exception: Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.List. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration.; nested exception is <openjpa-2.3.0-r422266:1540826 nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.Collection. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration.
        Hide
        Mark Struberg added a comment -

        would be happy about a review. The latest sources from trunk and 2.3.x should both fix the issues.

        Show
        Mark Struberg added a comment - would be happy about a review. The latest sources from trunk and 2.3.x should both fix the issues.
        Hide
        ASF subversion and git services added a comment -

        Commit 1536453 from Mark Struberg in branch 'openjpa/branches/2.3.x'
        [ https://svn.apache.org/r1536453 ]

        OPENJPA-2018 fix handling of Arrays in Select IN statements

        Show
        ASF subversion and git services added a comment - Commit 1536453 from Mark Struberg in branch 'openjpa/branches/2.3.x' [ https://svn.apache.org/r1536453 ] OPENJPA-2018 fix handling of Arrays in Select IN statements
        Hide
        ASF subversion and git services added a comment -

        Commit 1536439 from Mark Struberg in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1536439 ]

        OPENJPA-2018 fix handling of Arrays in Select IN statements

        Show
        ASF subversion and git services added a comment - Commit 1536439 from Mark Struberg in branch 'openjpa/trunk' [ https://svn.apache.org/r1536439 ] OPENJPA-2018 fix handling of Arrays in Select IN statements
        Hide
        Mark Struberg added a comment -

        also add a few tests for JPQL to make sure we broke nothing.
        Wonder if this did work in the past ...

        Show
        Mark Struberg added a comment - also add a few tests for JPQL to make sure we broke nothing. Wonder if this did work in the past ...
        Hide
        Mark Struberg added a comment -

        Romain, your latest patch uses Literal.TYPE_COLLECTION for arrays.
        I fear this is not a valid approach as the other direction also is used in our codebase.

        e.g. in criteria.Expressions line 1486 (toKernelExpression) there is still

        .. else if (((Literal)val2).getParseType() == Literal.TYPE_COLLECTION) {
             Collection coll = (Collection)((Literal)val2).getValue();
        

        Of course we can just do n instanceof but I think this is kind of dirty. I wonder whether we should introduce a dedicated Literal.TYPE_ARRAY to make the code cleaner.

        Show
        Mark Struberg added a comment - Romain, your latest patch uses Literal.TYPE_COLLECTION for arrays. I fear this is not a valid approach as the other direction also is used in our codebase. e.g. in criteria.Expressions line 1486 (toKernelExpression) there is still .. else if (((Literal)val2).getParseType() == Literal.TYPE_COLLECTION) { Collection coll = (Collection)((Literal)val2).getValue(); Of course we can just do n instanceof but I think this is kind of dirty. I wonder whether we should introduce a dedicated Literal.TYPE_ARRAY to make the code cleaner.
        Hide
        ASF subversion and git services added a comment -

        Commit 1536036 from Albert Lee in branch 'openjpa/branches/2.3.x'
        [ https://svn.apache.org/r1536036 ]

        OPENJPA-2018 add svn:eol-style=native property

        Show
        ASF subversion and git services added a comment - Commit 1536036 from Albert Lee in branch 'openjpa/branches/2.3.x' [ https://svn.apache.org/r1536036 ] OPENJPA-2018 add svn:eol-style=native property
        Hide
        ASF subversion and git services added a comment -

        Commit 1535839 from Mark Struberg in branch 'openjpa/branches/2.3.x'
        [ https://svn.apache.org/r1535839 ]

        OPENJPA-2018 improve unit test setup

        Show
        ASF subversion and git services added a comment - Commit 1535839 from Mark Struberg in branch 'openjpa/branches/2.3.x' [ https://svn.apache.org/r1535839 ] OPENJPA-2018 improve unit test setup
        Hide
        ASF subversion and git services added a comment -

        Commit 1535838 from Mark Struberg in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1535838 ]

        OPENJPA-2018 improve unit test setup

        Show
        ASF subversion and git services added a comment - Commit 1535838 from Mark Struberg in branch 'openjpa/trunk' [ https://svn.apache.org/r1535838 ] OPENJPA-2018 improve unit test setup
        Hide
        Mark Struberg added a comment -

        agree, slipped through. Will fix it.

        Show
        Mark Struberg added a comment - agree, slipped through. Will fix it.
        Hide
        Rick Curtis added a comment -

        Mark Struberg – It looks like you committed an earlier version of Romain's patch, but I still have a couple issues with it (the latest version).

        + "javax.persistence.provider", PersistenceProviderImpl.class.getName(),
        + "openjpa.RuntimeUnenhancedClasses", "supported",
        + "openjpa.ConnectionURL", "jdbc:derby:memory:openjpa2018;create=true",
        + "openjpa.ConnectionDriverName", "org.apache.derby.jdbc.EmbeddedDriver");
        + em = emf.createEntityManager();

        #1) RuntimeUnenhancedClasses is a feature we always tell users to stay away from. We should NEVER condone it's usage. It is buggy, and should have never made it into the codebase.

        #2) Don't specify database connection details in the test setup method. These properties will be passed in at runtime via Maven, or if running in Eclipse, via SystemProperties.

        #3) You shouldn't need to specify the persistence provider implementation class.

        Show
        Rick Curtis added a comment - Mark Struberg – It looks like you committed an earlier version of Romain's patch, but I still have a couple issues with it (the latest version). + "javax.persistence.provider", PersistenceProviderImpl.class.getName(), + "openjpa.RuntimeUnenhancedClasses", "supported", + "openjpa.ConnectionURL", "jdbc:derby:memory:openjpa2018;create=true", + "openjpa.ConnectionDriverName", "org.apache.derby.jdbc.EmbeddedDriver"); + em = emf.createEntityManager(); #1) RuntimeUnenhancedClasses is a feature we always tell users to stay away from. We should NEVER condone it's usage. It is buggy, and should have never made it into the codebase. #2) Don't specify database connection details in the test setup method. These properties will be passed in at runtime via Maven, or if running in Eclipse, via SystemProperties. #3) You shouldn't need to specify the persistence provider implementation class.
        Hide
        ASF subversion and git services added a comment -

        Commit 1535816 from Mark Struberg in branch 'openjpa/branches/2.3.x'
        [ https://svn.apache.org/r1535816 ]

        OPENJPA-2018 correctly handle select IN with arrays

        txs to rmannibucau for the patch.
        Applied with minor changes

        Show
        ASF subversion and git services added a comment - Commit 1535816 from Mark Struberg in branch 'openjpa/branches/2.3.x' [ https://svn.apache.org/r1535816 ] OPENJPA-2018 correctly handle select IN with arrays txs to rmannibucau for the patch. Applied with minor changes
        Hide
        Romain Manni-Bucau added a comment -

        missingpatch.patch is a diff between what Mark applied and what was in my last patch to handle array too in all cases

        Show
        Romain Manni-Bucau added a comment - missingpatch.patch is a diff between what Mark applied and what was in my last patch to handle array too in all cases
        Hide
        ASF subversion and git services added a comment -

        Commit 1535760 from Mark Struberg in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1535760 ]

        OPENJPA-2018 correctly handle select IN with arrays

        txs to rmannibucau for the patch.
        Applied with minor changes

        Show
        ASF subversion and git services added a comment - Commit 1535760 from Mark Struberg in branch 'openjpa/trunk' [ https://svn.apache.org/r1535760 ] OPENJPA-2018 correctly handle select IN with arrays txs to rmannibucau for the patch. Applied with minor changes
        Hide
        Romain Manni-Bucau added a comment -

        Same patch with a better handling of array in all cases (was working with the sample of Olivier but not with the bind using a name/position case)

        Show
        Romain Manni-Bucau added a comment - Same patch with a better handling of array in all cases (was working with the sample of Olivier but not with the bind using a name/position case)
        Hide
        Rick Curtis added a comment -

        When developing/running unit tests in your IDE you can use the javaagent and when the tests are run with maven the Entities will be enhanced as part of the build.

        Show
        Rick Curtis added a comment - When developing/running unit tests in your IDE you can use the javaagent and when the tests are run with maven the Entities will be enhanced as part of the build.
        Hide
        Romain Manni-Bucau added a comment -

        updating the test.

        Note: I kept supported for runtime unenhancedclass cause ATM the parent is not hacking classloaders to do enhancement in test (it relies on mvn AFAIK) so it is not easy to work on tests without it and for this test this is enough.

        Show
        Romain Manni-Bucau added a comment - updating the test. Note: I kept supported for runtime unenhancedclass cause ATM the parent is not hacking classloaders to do enhancement in test (it relies on mvn AFAIK) so it is not easy to work on tests without it and for this test this is enough.
        Hide
        Rick Curtis added a comment -

        My only comment on the patch is in regards to the unit test. For future reference, don't use openjpa.RuntimeUnenhancedClasses. That is a very buggy feature and we really should be discouraging usage of it. Also try to follow the test case pattern outlined by SingleEMTestCase / SingleEMFTestCase. This way you'll get enhancement for free, and the test will work across all of the different supported databases.

        Show
        Rick Curtis added a comment - My only comment on the patch is in regards to the unit test. For future reference, don't use openjpa.RuntimeUnenhancedClasses. That is a very buggy feature and we really should be discouraging usage of it. Also try to follow the test case pattern outlined by SingleEMTestCase / SingleEMFTestCase. This way you'll get enhancement for free, and the test will work across all of the different supported databases.
        Hide
        Romain Manni-Bucau added a comment - - edited

        Proposed patch

        @Olivier: thank you for the sample, it really helps

        Show
        Romain Manni-Bucau added a comment - - edited Proposed patch @Olivier: thank you for the sample, it really helps
        Hide
        Oliver Gierke added a comment -

        Sample project to show binding fail with both arrays and a collection. Run mvn clean test to see it fail.

        Show
        Oliver Gierke added a comment - Sample project to show binding fail with both arrays and a collection. Run mvn clean test to see it fail.
        Hide
        Romain Manni-Bucau added a comment -

        Hi Olivier,

        do you care attaching a sample with a failling test reproducing it? (seems you have it not that far)

        Show
        Romain Manni-Bucau added a comment - Hi Olivier, do you care attaching a sample with a failling test reproducing it? (seems you have it not that far)
        Hide
        Oliver Gierke added a comment -

        Any update here? This ticket is more than 2 years old and effectively a violation of the spec.

        Show
        Oliver Gierke added a comment - Any update here? This ticket is more than 2 years old and effectively a violation of the spec.
        Hide
        Oliver Gierke added a comment -

        It does not seem to work with collections as well:

        CriteriaBuilder builder = em.getCriteriaBuilder();

        CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class);
        Root<User> root = criteriaQuery.from(User.class);
        criteriaQuery.where(root.<Integer> get("id").in(builder.parameter(Collection.class)));

        TypedQuery<User> query = em.createQuery(criteriaQuery);
        for (Parameter parameter : query.getParameters())

        { query.setParameter(parameter, Arrays.asList(1, 2)); }

        List<User> resultList = query.getResultList();

        This fails with:

        <openjpa-2.0.0-r422266:935683 nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.Collection. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration.

        Tried against 2.0 as well as 2.1. Don't quite get why OpenJPA tries to compare the integer against the collection instead of its values.

        Show
        Oliver Gierke added a comment - It does not seem to work with collections as well: CriteriaBuilder builder = em.getCriteriaBuilder(); CriteriaQuery<User> criteriaQuery = builder.createQuery(User.class); Root<User> root = criteriaQuery.from(User.class); criteriaQuery.where(root.<Integer> get("id").in(builder.parameter(Collection.class))); TypedQuery<User> query = em.createQuery(criteriaQuery); for (Parameter parameter : query.getParameters()) { query.setParameter(parameter, Arrays.asList(1, 2)); } List<User> resultList = query.getResultList(); This fails with: <openjpa-2.0.0-r422266:935683 nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Filter invalid. Cannot compare field id of type java.lang.Integer to value of type java.util.Collection. Numeric comparisons must be between numeric types only. To enable such comparisons for backwards-compatibility, add "QuotedNumbersInQueries=true" to the org.apache.openjpa.Compatibility setting in your configuration. Tried against 2.0 as well as 2.1. Don't quite get why OpenJPA tries to compare the integer against the collection instead of its values.

          People

          • Assignee:
            Mark Struberg
            Reporter:
            Oliver Gierke
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development