spring-projects
Consider `NULLS` precedence using `Sort`
Vladimir Domashnev opened DATAJPA-925 and commented
As of 1.10.2, toJpaOrder(Order, Root<?>, CriteriaBuilder) method of org.springframework.data.jpa.repository.query.QueryUtils ignores null precedence, that is provided by org.springframework.data.domain.Sort.Order nullHandling propery, which is present since 1.7.x. It is used by org.springframework.data.jpa.repository.support.SimpleJpaRepository.getQuery(Specification...), thus affecting many methods of org.springframework.data.jpa.repository.JpaRepository interface
Affects: 1.10.2 (Hopper SR2)
Reference URL: https://github.com/spring-projects/spring-data-jpa/blob/master/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
Issue Links:
- DATAJPA-754 NullHandling.(NULLS_LAST and NULLS_FIRST) does not work ("is duplicated by")
- DATAJPA-1302 NullHandling.(NULLS_LAST and NULLS_FIRST) does not work ("is duplicated by")
- DATAJPA-1725 Using nullsFirst() or nullsLast() on Sort and JpaSort has no effect on database query ("is duplicated by")
- DATACMNS-904 NullHandling not working ("is duplicated by")
- DATAJPA-825 Take into account org.springframework.data.domain.Sort.NullHandling for JPA Criteria
4 votes, 11 watchers
Jens Schauder commented
I have a PR in the working. The basic thing works, including unit tests, but I have a couple of questions:
-
I'm not sure if
NULLS LASTandNULLS FIRSTis supported by all databases. How should that get handled? -
The Unit tests, test that the sql statement gets created as expected, but obviously not, that it performs as expected. Should I add an integration test for that?
-
Should I provide an example using this feature? Maybe that would serve as an integration test?
current state of the PR to be is here: https://github.com/schauder/spring-data-jpa
Oliver Drotbohm commented
I am kind of torn on this one (and propbably should've expressed that earlier). The reason for that is while we could probably rely on the persistence providers supporting that non-JPQL expression, there's nothing we can do about the lack of support for that in the criteria API. Query derivation is based on that so that we're gonna hit a wall there the latest.
I think I even filed a ticket for JPA already but can't currently find it as I am on a thumb keyboard. I have not yet investigated whether there's a way to maybe even hand that additional fact to the persistence provider via a query hint maybe
Benjamin Demarteau commented
Just dropping the mentioned link here: https://github.com/javaee/jpa-spec/issues/76
loicrouchon commented
Criteria API does actually provide one way to handle this problem. You can do it by using a default value for the NULL values in the ORDER BY clause.
You can then define an order using
CriteriaBuilder cb;
Root<T> root;
T defaultValue;
javax.persistence.criteria.Order order= cb.desc(cb.coalesce(root.get("myAttribute"), defaultValue));
The null last behaviour can then be used by providing as a default value the maximum or minimum value for type T depending of the ordering direction. Examples with a an integer attribute:
- direction: ascending, null lasts:
-
cb.asc(cb.coalesce(root.get("myAttribute"), Integer.MAX_VALUE));
-
- direction: descending, null lasts:
-
cb.desc(cb.coalesce(root.get("myAttribute"), Integer.MIN_VALUE));
-
- direction: ascending, null first:
-
cb.asc(cb.coalesce(root.get("myAttribute"), Integer.MIN_VALUE));
-
- direction: descending, null first:
-
cb.asc(cb.coalesce(root.get("myAttribute"), Integer.MAX_VALUE));
-
So a way to provide this NULL last/first handling feature would be to provide in org.springframework.data.domain.Sort.Order an optional default value for NULL comparisons and rely on the javax.persistence.criteria.CriteriaBuilder#coalesce for the implementation part.
User code to have a NULL last behaviour on a Date attribute could then look like this:
Sort sort = new Sort(new Order(Direction.ASC, "dateAttribute", new Date(Long.MAX_VALUE));
It's a change of paradigm, but it is more flexible than having NULL first or last as it allows to properly model the equivalent value of a NULL
Jens Schauder commented
While this is a valid workaround on JPA level it does not work for JPA because we need to know the MAX_VAlUE/MIN_VALUE equivalent for the type of a property. Which is tricky for Date or String and becomes impossible for custom types like Email.
Therefore this will have to wait until there is proper support from JPA for this
Ruslan Stelmachenko commented
As I already mentioned in DATAJPA-825 there is a way to do it consistently:
javax.persistence.criteria.Order nullsLastOrder = builder.asc(
builder.selectCase()
.when(builder.isNotNull(root.get("someproperty")), 0)
.otherwise(1));
We can use CASE expression and insert it before each actual ORDER expression (so, each DATAJPA Order instance with NullHandling other then NullHandling.NATIVE will be transformed into two criteria Order instances). That way we don't need to know MAX_VALUE/MIN_VALUE equivalent for the type of a property.
I think it should work.
But this solution (as any other solution that emulates native DB functionality) have a serious drawback: it prevents to use indexes for ORDER BY, at least in Mysql.
It results in something like:
ORDER BY CASE WHEN (t.prop IS NOT NULL) THEN CAST(0 AS INTEGER) ELSE CAST(1 AS INTEGER) END, t.prop ASC
instead of just
ORDER BY t.prop ASC
As I know, if you have an index on t.prop, then Mysql will use it in the second ORDER BY expression, but will not use it in the first ORDER BY expression. This can lead to a serious performance penalty.
But to be honest, this is the only way to implement it in Mysql (it doesn't support NULLS FIRST/LAST). So if user requires this sort order, it's acceptable (because there is no other way to do it anyway).
But for databases that support NULLS FIRST/LAST, it is much better to implement this in native way, of course.
I agree that this is responsibility of JPA provider to generate right queries for each DB. So, without support from Criteria API it's hard to implement this performant on DATAJPA side. It is hard to implement this right even on JPA-provider side, I think. Because there is so many DBs with different level of support for this feature, unfortunately. Maybe that is the reason why this is not included in the JPA standard in the first place..
Finally, the JPA spec issue has been addressed so we can proceed here for the JPA 3.2 spec. Additionally, we should extend our parsers via #3108
Actually, there are multiple changes coming in JPA 3.2, all tracked via #3136