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

Consider NULLS precedence using Sort #1280

Open
spring-projects-issues opened this issue Jul 5, 2016 · 10 comments
Open

Consider NULLS precedence using Sort #1280

spring-projects-issues opened this issue Jul 5, 2016 · 10 comments
Assignees
Labels
in: core Issues in core support type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

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

@spring-projects-issues
Copy link
Author

Jens Schauder commented

I have a PR in the working. The basic thing works, including unit tests, but I have a couple of questions:

  1. I'm not sure if NULLS LAST and NULLS FIRST is supported by all databases. How should that get handled?

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

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

@spring-projects-issues
Copy link
Author

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

@spring-projects-issues
Copy link
Author

Benjamin Demarteau commented

Just dropping the mentioned link here: https://github.com/javaee/jpa-spec/issues/76

@spring-projects-issues
Copy link
Author

Jens Schauder commented

The issue is now here: jakartaee/persistence#76

@spring-projects-issues
Copy link
Author

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

@spring-projects-issues
Copy link
Author

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

@spring-projects-issues
Copy link
Author

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

@gregturn
Copy link
Contributor

This is pending jakartaee/persistence#76.

@mp911de mp911de added type: enhancement A general enhancement and removed type: bug A general bug status: blocked An issue that's blocked on an external project change labels Aug 10, 2023
@mp911de mp911de changed the title SimpleJpaRepository.getQuery() ignores null precedence [DATAJPA-925] Consider NULLS precedence Aug 10, 2023
@mp911de
Copy link
Member

mp911de commented Aug 10, 2023

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

@mp911de mp911de changed the title Consider NULLS precedence Consider NULLS precedence using Sort Aug 10, 2023
@gregturn
Copy link
Contributor

gregturn commented Sep 8, 2023

Actually, there are multiple changes coming in JPA 3.2, all tracked via #3136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants