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
          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:
              31 Start watching this issue

              Dates

              • Created:
                Updated: