Uploaded image for project: 'Spring Data JPA'
  1. Spring Data JPA
  2. DATAJPA-209

Improve handling of null query method parameter values

    Details

    • Type: Improvement
    • Status: Investigating
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 1.1 RC1
    • Fix Version/s: None
    • Component/s: None

      Description

      In 1.1.0.RC1 and prior, query methods expect non-null values. If a null value is passed in to a query method, the JPQL generated includes an "= NULL" condition, which is always false.

      SD JPA supports query method keyword IsNull, which allows for testing explicitly whether a value is null. This is ok, but fails to meet our requirement of using null parameter values to indicate that that parameter should be ignored and not included in the query.

      Here's an example. Suppose I have a Foo that references a Bar and a Goo, and I want to create a query that finds me any Foo instances that reference a given Bar and/or Goo. The query method would look like this:

      public interface FooRepository extends JpaRepository<Foo> {
       
        List<Foo> findByBarAndGoo(Bar bar, Goo goo);
      }

      If this method is called with a non-null values for both parameters, everything works fine. However, if you pass null for either parameter, no Foo instances are found because = NULL is always false. One alternative is for the author to write custom, boilerplate method implementations that handle null instances as desired. Another alternative is to write a collection of methods representing all of the permutations of the nullable parameters, which doesn't really scale well past two or three parameters:

      public interface FooRepository extends JpaRepository<Foo> {
       
        List<Foo> findByBarAndGoo(Bar bar, Goo goo);
        List<Foo> findByBar(Bar bar);
        List<Foo> findByGoo(Goo goo);
      }

      This issue represents a request to improve this situation.

      Consider a new enum & annotation:

      public enum NullBehavior {
      	EQUALS, IS, IGNORED
      }
       
      @Retention(RetentionPolicy.RUNTIME)
      @Target({ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER})
      public @interface NullMeans {
      	NullBehavior value() default NullBehavior.EQUALS;
      }

      With this annotation, SD JPA would let the author decide how to behave when null parameters are encountered. In the absence of the annotation, the current default behavior (= NULL) would apply. If the author uses @NullMeans(IS), then SD JPA will produce an IS NULL clause. If they use @NullMeans(IGNORED), then SD JPA does not include a clause for the given parameter.

      Now, reconsider the Foo example. I now have a flexible way of specifying the queries I want.

      public interface FooRepository extends JpaRepository<Foo> {
       
        List<Foo> findByBarAndGoo(@NullMeans(IGNORED) Bar bar, @NullMeans(IGNORED) Goo goo);
      }

      This also scales well:

      public interface BlazRepository extends JpaRepository<Blaz> {
       
        @NullMeans(IGNORED) // applies to all parameters unless overriden by @NullMeans on parameter(s)
        List<Blaz> findByFooAndGooAndHooAndKooAndLoo(Foo foo, Goo goo, Hoo hoo, Koo koo, @NullMeans(IS) Loo loo);
      }

      I've also allowed @NullMeans to be placed on the interface as well, which would provide a default for all parameters on all query methods defined in the interface. I would imagine that many folks would use @NullMeans(IGNORED) at the interface level since it's so practical.

        Issue Links

          Activity

          Hide
          matthewadams Matthew T. Adams added a comment -

          JIRA jacked up code formatting. BlazRepository should be:

          public interface BlazRepository extends JpaRepository<Blaz> {
          @NullMeans(IGNORED)
          List<Blaz> findByFooAndGooAndHooAndKooAndLoo(Foo foo, Goo goo, Hoo hoo, Koo koo, @NullMeans(IS) Loo loo);
          }

          Show
          matthewadams Matthew T. Adams added a comment - JIRA jacked up code formatting. BlazRepository should be: public interface BlazRepository extends JpaRepository<Blaz> { @NullMeans(IGNORED) List<Blaz> findByFooAndGooAndHooAndKooAndLoo(Foo foo, Goo goo, Hoo hoo, Koo koo, @NullMeans(IS) Loo loo); }
          Hide
          olivergierke Oliver Gierke added a comment -

          Are you sure you can reproduce this (SD JPA using = NULL) against 1.1.0.RC1? We had DATAJPA-121 reported and resolved for 1.1.0.RC1 so that SD JPA should now correctly use IS NULL for the query if null is provided as parameter. So I don't think we create invalid queries for null parameters anymore.

          Beyond that I'd like to see whether there's community feedback and votes on this one as this would introduce quite a bit of complexity to the configuration and implementation actually.

          Show
          olivergierke Oliver Gierke added a comment - Are you sure you can reproduce this (SD JPA using = NULL ) against 1.1.0.RC1? We had DATAJPA-121 reported and resolved for 1.1.0.RC1 so that SD JPA should now correctly use IS NULL for the query if null is provided as parameter. So I don't think we create invalid queries for null parameters anymore. Beyond that I'd like to see whether there's community feedback and votes on this one as this would introduce quite a bit of complexity to the configuration and implementation actually.
          Hide
          matthewadams Matthew T. Adams added a comment - - edited

          FYI, I'm unable to repro the "= NULL" v. "IS NULL" bug so far using the showcase in the SD JPA example after updating its POMs to look similar to mine. I'm digging further into my POMs to see what might be going on.

          Show
          matthewadams Matthew T. Adams added a comment - - edited FYI, I'm unable to repro the "= NULL" v. "IS NULL" bug so far using the showcase in the SD JPA example after updating its POMs to look similar to mine. I'm digging further into my POMs to see what might be going on.
          Hide
          matthewadams Matthew T. Adams added a comment -

          Assuming that the "IS NULL" parameter handling is fixed, then that would warrant the removal of the EQUALS value in the proposed NullBehavior enum, leaving only IS and IGNORED.

          Show
          matthewadams Matthew T. Adams added a comment - Assuming that the "IS NULL" parameter handling is fixed, then that would warrant the removal of the EQUALS value in the proposed NullBehavior enum, leaving only IS and IGNORED.
          Hide
          matthewadams Matthew T. Adams added a comment -

          Ok, just confirmed that I'm indeed seeing the behavior that was fixed as a result of DATAJPA-121. I must've first seen this behavior with a version of SD JPA prior to 1.1.0.RC1 (probably 1.1.0.M1 or M2).

          Show
          matthewadams Matthew T. Adams added a comment - Ok, just confirmed that I'm indeed seeing the behavior that was fixed as a result of DATAJPA-121 . I must've first seen this behavior with a version of SD JPA prior to 1.1.0.RC1 (probably 1.1.0.M1 or M2).
          Hide
          olivergierke Oliver Gierke added a comment -

          Thanks for confirmation, Adam. Just wanted to make sure we don't have a glitch preventing the fix for DATAJPA-121 working. You proposal is definitely adding value on top of the fix, so I'll keep this one around.

          Show
          olivergierke Oliver Gierke added a comment - Thanks for confirmation, Adam. Just wanted to make sure we don't have a glitch preventing the fix for DATAJPA-121 working. You proposal is definitely adding value on top of the fix, so I'll keep this one around.
          Hide
          tejo_ak tejo a kusuma added a comment -

          this is definitely a very important feature to sd-jpa, because I think that this will simplify complex work regarding dynamic query. so please consider to add this feature to spring data jpa

          Show
          tejo_ak tejo a kusuma added a comment - this is definitely a very important feature to sd-jpa, because I think that this will simplify complex work regarding dynamic query. so please consider to add this feature to spring data jpa
          Hide
          santharamselva s.selvakumar added a comment - - edited

          Hi All,
          I don't to whether right way to ask here.
          In query I have passed my object itself in Criteria, "where key" as a "is" condition. Its dig the object and get the value but when object field value is "null" field, its making query as "null" values instead of skip query parameter, which leads to wrong query. Is it possible to over come this problem, or can expect as a improvement.

          Show
          santharamselva s.selvakumar added a comment - - edited Hi All, I don't to whether right way to ask here. In query I have passed my object itself in Criteria, "where key" as a "is" condition. Its dig the object and get the value but when object field value is "null" field, its making query as "null" values instead of skip query parameter, which leads to wrong query. Is it possible to over come this problem, or can expect as a improvement.
          Hide
          greatthor Uwe Allner added a comment -

          It is important to note, that you must not annotate your finder with a @Query. Because in that case null values are still inserted as "field = null" into the generated SQL. By just using the name of the finder to generate the query it all works fine...

          Show
          greatthor Uwe Allner added a comment - It is important to note, that you must not annotate your finder with a @Query. Because in that case null values are still inserted as "field = null" into the generated SQL. By just using the name of the finder to generate the query it all works fine...
          Hide
          mlubavin Michael Lubavin added a comment -

          Hi, any update on this feature? I am just starting with Spring Data JPA, and it seems strange that I have to implement JPQL just to be able to have optional parameters (and I still haven't wrapped my head around optional IN clauses).

          Show
          mlubavin Michael Lubavin added a comment - Hi, any update on this feature? I am just starting with Spring Data JPA, and it seems strange that I have to implement JPQL just to be able to have optional parameters (and I still haven't wrapped my head around optional IN clauses).
          Hide
          wojciech.krak Wojciech Krak added a comment -

          I also find this feature useful. It would be nice if it would be working in every spring-data subproject(now I'm using spring-data-mongo)

          Show
          wojciech.krak Wojciech Krak added a comment - I also find this feature useful. It would be nice if it would be working in every spring-data subproject(now I'm using spring-data-mongo)
          Hide
          james.king.zj James King added a comment -

          We can simply generate the query based on the query method name and the passed in parameters and ignore the case if the query is generated based on @Query.

          Show
          james.king.zj James King added a comment - We can simply generate the query based on the query method name and the passed in parameters and ignore the case if the query is generated based on @Query.
          Hide
          jasonfungsing Jason Fung added a comment -

          Agree with Wojeciech, can you make this available for all spring-data subproject.

          Show
          jasonfungsing Jason Fung added a comment - Agree with Wojeciech, can you make this available for all spring-data subproject.
          Hide
          nickvdh Nick Vanderhoven added a comment - - edited

          I agree and was strongly suprised this is not the default behaviour. Every mainstream project has a search form with multiple fields that can be left empty. First we get a lot of advantages and concise code and then we have to throw them away and go write everything ourselves. This would be a big enabler for adopters.

          Show
          nickvdh Nick Vanderhoven added a comment - - edited I agree and was strongly suprised this is not the default behaviour. Every mainstream project has a search form with multiple fields that can be left empty. First we get a lot of advantages and concise code and then we have to throw them away and go write everything ourselves. This would be a big enabler for adopters.
          Hide
          Vladimir Krasikov Vladimir Krasikov added a comment - - edited

          I'm waiting for this improvement to. Tell please, there are any chance that you will make this improvement?
          Maybe you can implement the behaviour like in Spring Web MVC. I talk about org.springframework.web.bind.annotation.RequestParam with 'required' and 'defaultValue' parameters.

          Show
          Vladimir Krasikov Vladimir Krasikov added a comment - - edited I'm waiting for this improvement to. Tell please, there are any chance that you will make this improvement? Maybe you can implement the behaviour like in Spring Web MVC. I talk about org.springframework.web.bind.annotation.RequestParam with 'required' and 'defaultValue' parameters.
          Hide
          gionn Giovanni Toraldo added a comment -

          +1 for this, we are still using hand-made queries to handle this kind of situations.

          Show
          gionn Giovanni Toraldo added a comment - +1 for this, we are still using hand-made queries to handle this kind of situations.
          Hide
          RodrigoDev Rodrigo Quesada added a comment -

          I think this is a good feature to have but I don't believe having a null value passed-in to a method is a good idea for specifying the corresponding field should not be included in the query. The semantic value of the null keyword is very poor, so I don't even think is okay to use it with a parameter annotation as suggested here. A better option would be to use an Optional class (be it Java 8 or a previous version) to specify a parameter is optional, even if it means not having the convenience of being able to define—in a global manner—that all parameters for a given class are optional (such as you would do by annotating that class or one of its interfaces). Also, it would be great to have this functionality available across all SD and not only for JPA.

          Show
          RodrigoDev Rodrigo Quesada added a comment - I think this is a good feature to have but I don't believe having a null value passed-in to a method is a good idea for specifying the corresponding field should not be included in the query. The semantic value of the null keyword is very poor, so I don't even think is okay to use it with a parameter annotation as suggested here. A better option would be to use an Optional class (be it Java 8 or a previous version) to specify a parameter is optional, even if it means not having the convenience of being able to define—in a global manner—that all parameters for a given class are optional (such as you would do by annotating that class or one of its interfaces). Also, it would be great to have this functionality available across all SD and not only for JPA.
          Hide
          olivergierke Oliver Gierke added a comment -

          While I can see this being useful I have a few conceptual issues with this as well:

          1. The mechanism basically requires rebuilding the queries for every invocation as optional predicates have to be left out. While this might be acceptable, it hugely complicates the implementation of the query creation, validation, parameter binding etc.
          2. How is this supposed to work with declared queries? They can become pretty complex and I don't really want to dive into manipulating them to potentially remove parts of them.
          3. I think a mechanism like this would create incentives to write overly complex query methods (i.e. optionalize everything) despite the fact that there already is a mechanism to dynamically combine predicates: Specifications and Querydsl. The last example Matthew gave is a good one for exactly that (even picturing more reasonable property names). In this case, I'd definitely recommend using a manually defined query, which then brings us back to question 2.

          I am generally inclined not to add something half baked, so I guess this might get back on our table for a 2.0 (probably on top of Spring 5 in 2016) when we revise the usage of Optional in the core repository abstractions anyway. But for now I don't think we're going to do anything about it.

          Show
          olivergierke Oliver Gierke added a comment - While I can see this being useful I have a few conceptual issues with this as well: 1. The mechanism basically requires rebuilding the queries for every invocation as optional predicates have to be left out. While this might be acceptable, it hugely complicates the implementation of the query creation, validation, parameter binding etc. 2. How is this supposed to work with declared queries? They can become pretty complex and I don't really want to dive into manipulating them to potentially remove parts of them. 3. I think a mechanism like this would create incentives to write overly complex query methods (i.e. optionalize everything) despite the fact that there already is a mechanism to dynamically combine predicates: Specifications and Querydsl. The last example Matthew gave is a good one for exactly that (even picturing more reasonable property names). In this case, I'd definitely recommend using a manually defined query, which then brings us back to question 2. I am generally inclined not to add something half baked, so I guess this might get back on our table for a 2.0 (probably on top of Spring 5 in 2016) when we revise the usage of Optional in the core repository abstractions anyway. But for now I don't think we're going to do anything about it.

            People

            • Assignee:
              olivergierke Oliver Gierke
              Reporter:
              matthewadams Matthew T. Adams
              Last updater:
              Oliver Gierke
            • Votes:
              34 Vote for this issue
              Watchers:
              30 Start watching this issue

              Dates

              • Created:
                Updated: