Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of null query method parameter values [DATAJPA-209] #622

Open
spring-projects-issues opened this issue May 4, 2012 · 30 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented May 4, 2012

Matthew T. Adams opened DATAJPA-209 and commented

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


Affects: 1.1 RC1

Issue Links:

  • DATACMNS-1319 Suggestion: change interpretation of Optionals as parameters in Spring Data JPA repositories interfaces
    ("is duplicated by")

  • DATAJPA-121 Query parameter is null,it still use equals(=) to compare

  • DATACMNS-490 Add support for optional query method parameters

77 votes, 72 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 4, 2012

Matthew T. Adams commented

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);
}

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 6, 2012

Oliver Drotbohm commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 8, 2012

Matthew T. Adams commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 8, 2012

Matthew T. Adams commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 8, 2012

Matthew T. Adams commented

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)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 8, 2012

Oliver Drotbohm commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 5, 2012

tejo a kusuma commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 12, 2013

s.selvakumar commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2014

Uwe Allner commented

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...

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 24, 2014

Michael Lubavin commented

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)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 24, 2014

Wojciech Krak commented

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)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 26, 2014

James King commented

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.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 24, 2015

Jason Feng commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 10, 2015

Nick Vanderhoven commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 28, 2015

Vladimir Krasikov commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 16, 2015

Giovanni Toraldo commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 16, 2015

Rodrigo Quesada commented

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 17, 2015

Oliver Drotbohm commented

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.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 2, 2016

Mohammad commented

I think this is a must... almost more than 50 pc of our queries in our application has this clause as '(x = :x or :x is null) and ....' For this reason currently we have to handcraft all these queries

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 19, 2016

Mikhail Mikhaylenko commented

Maybe there is a possibility to add Optional keyword to query derivation mechanism?
If this keyword is present before parameter name, then null value will force skip of adding conditions with this parameter to query.

public interface FooRepository extends JpaRepository<Foo> {
  List<Foo> findByOptionalBarAndGoo(Bar bar, Goo goo);
  // or even like this:
  List<Foo> findByOptionalBarAndGoo(Optional<Bar> optBar, Goo goo); // which maybe a bad practice
}
@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 19, 2016

Oliver Drotbohm commented

Optional is already supported but in the way it's intended to be used: instances of Optional must never be null themselves, Optional.empty() is basically treated like null downstream

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 5, 2016

John Day commented

This feature would be of great help of a particular situation: processing query conditions. Say, there are a number of filter conditions in a personnel query form, like name, department, office location and etc, you got the picture. Users are not required to fill each and every input box, in fact, when the input box is left empty, it has a specific syntax meaning: not applying that filter in the query. Any field and any combination of fields can be empty, so it'd be an exhausting job to iterate every situation. Under a limited circumstances, Query By Example may be of help. However, when facing more variable query conditions like BETWEEN, LIKE and etc, Spring Data JPA is really weak and unproductive. At the moment, the only promising solution is Specification. I'll try to come up with a working example

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 5, 2016

Oliver Drotbohm commented

Have you looked at the Querydsl support? Query methods are not means to execute flexible predicates but well-known-in-advance queries. They trade off the flexibility to combine predicates (non-existant) with the ease you can create those. If you want to freely combine predicates, query methods is simply not the feature you want to use but — as you already realized — specifications or — less JPA bound — Querydsl

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 2, 2017

Jens Schauder commented

Regarding point #1 from above:
"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."

Not sure if this has to be true. an optional constraint could be coded like this

(property = :parameter or :parameter is Null)

or when nulls should mean is null

(property = :parameter or (:parameter is null and property is null))

This leaves the query constant, which our infrastructure and statement caches in the database will like. It might harm usage of indexes but for those cases where this matters, one might fall back use Querydsl as proposed

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 6, 2017

Jens Schauder commented

I came across this https://stackoverflow.com/questions/44376354/how-to-use-oracle-nvl-function-in-spring-data-repository-nativequery Stackoverflow question and others having similiar problems sometimes with exactly the kind of query I proposed above. Makes me wonder if this actually would work

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2020

Jens Schauder commented

We want to fix this, with a null argument essentially meaning is null.

This may be implemented in two ways: either by a complex condition (property = :parameter or (:parameter is null and property is null)) or by generating the query afresh depending on the arguments, possibly with cashing

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 24, 2020

FlaviusBurghila commented

Any news about this feature?

Thanks

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2020

Jens Schauder commented

As described in the last comment we decided that we want this to get implemented. But we have no plan on the timing

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 30, 2020

timguy commented

please keep also in mind all the findByIn (e.g.: findByFooIn(List<..) cases.

Because all the described statements like (property = :parameter or (:parameter is null and property is null)) would not apply here. At least when I use @Query with (:partnerIds is null the list expand to: (:partnerIds_0, :partnerIds_1 is null which is not a valid query

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 28, 2020

simonparadies commented

A valid and working @Query part for findByInPartnerIds would be "...(coalesce( null, :partnerIds ) = null or partnerId in :partnerIds )..."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants