Spring Data JPA
  1. Spring Data JPA
  2. DATAJPA-252

Pageable sorting by join property excludes null matches

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.2, 1.2 RC1
    • Fix Version/s: 1.2 GA
    • Component/s: None
    • Labels:
      None

      Description

      When using the PagingAndSortingRepository with a Pageable query, attempts to sort by the join property will exclude entities where the join condition is absent.

      From JSR-317 4.4.5.2:

      LEFT JOIN and LEFT OUTER JOIN are synonymous. They enable the retrieval of a set of entities where matching values in the join condition may be absent.

      According to standard SQL and JPA specs, the left outer join should always contain all entities from the "left" table even if the join condition is absent. Therefore, the exclusion of the "left" entities when there is no matching value in the "right" table seems incorrect.

      For example, given an entity "Person" with optional OneToOne relationship "Address" (where Address is the owner), any attempts to retrieve pages of Person entities from the repository that are ordered by an Address property will only return Person entities that have non-null Addresses. This is only a problem for Pageable and Sort queries; using the same @Query with a non-pageable/sort works as expected.

      // Includes Persons with no Addresses and sorts by address.city, as expected
      @Query("select p from Person p left outer join p.address address order by address.city")
      List<Person> findPersonOrderByCity();
      
      // Excludes Persons with no Addresses when sorting by an address property
      @Query("select p from Person p left outer join p.address address")
      Page<Person> findPerson(Pageable pageable);
      
      // Excludes Persons with no Addresses when sorting by an address property    
      @Query("select p from Person p left outer join p.address address")
      List<Person> findPerson(Sort sort);
      

      The resulting core sql statements (simplified slightly for brevity) are as follows; notice the additional FROM clause incorrectly added to the pageable query:

      @Query("select p from Person p left outer join p.address address order by address.city")
      List<Person> findPersonOrderByCity();
      select p.id, p.first_name, p.last_name from person p left outer join address a on p.id=a.person order by a.city
      
      @Query("select p from Person p left outer join p.address address")
      Page<Person> findPerson(Pageable pageable);
      select p.id, p.first_name, p.last_name from person p left outer join address a on p.id=a.person, address a2 where p.id=a2.person order by a2.city asc
      

      A full test case is attached. This has been tested with spring-data-jpa-1.1.1.RELEASE and hibernate-4.1.5.SP1.

        Activity

        Hide
        Shelley J. Baker added a comment -

        Thanks for resolving this so quickly. I took the snapshot for a spin and it works like a charm.

        For reference to anyone else running into this problem in the meantime, to workaround this issue, I just added some methods to the repository that explicitly define the "order by" in the query for the problem properties. In the service from where we invoke the repository method, I just check to see if the requested order is one of the problem properties and call the alternate repository method with a copy of the pageable.

        public Page<Person> findPerson(Pageable pageable) {
        
            Order order = pageable.getSort() != null ? pageable.getSort().iterator().next() : null;
            if (order != null && "address.city".equals(order.getProperty())) {
                Pageable orderlessPageable = new PageRequest(pageable.getPageNumber(), pageable.getPageSize());
                final Page<Person> pages;
                if (order.isAscending()) {
                    pages = personRepository.findPersonOrderByCityAsc(orderlessPageable);
                } else {
                    pages = personRepository.findPersonOrderByCityDesc(orderlessPageable);
                }
                return new PageImpl<Person>(pages.getContent(), pageable, pages.getTotalElements());
            }
            
            return personRepository.findPerson(pageable);
        }
        

        This isn't a robust solution but was a simple and feasible workaround in my case because we currently only need to support this type of sorting for one specific use case at this time. Also, the repository and code is internal so there isn't currently a need for us to be any more robust.

        This might not work for everyone running into this problem but was a reasonable workaround for our use case, which we'll use until spring-data-jpa 1.2 is released with this fix.

        Show
        Shelley J. Baker added a comment - Thanks for resolving this so quickly. I took the snapshot for a spin and it works like a charm. For reference to anyone else running into this problem in the meantime, to workaround this issue, I just added some methods to the repository that explicitly define the "order by" in the query for the problem properties. In the service from where we invoke the repository method, I just check to see if the requested order is one of the problem properties and call the alternate repository method with a copy of the pageable. public Page<Person> findPerson(Pageable pageable) { Order order = pageable.getSort() != null ? pageable.getSort().iterator().next() : null ; if (order != null && "address.city" .equals(order.getProperty())) { Pageable orderlessPageable = new PageRequest(pageable.getPageNumber(), pageable.getPageSize()); final Page<Person> pages; if (order.isAscending()) { pages = personRepository.findPersonOrderByCityAsc(orderlessPageable); } else { pages = personRepository.findPersonOrderByCityDesc(orderlessPageable); } return new PageImpl<Person>(pages.getContent(), pageable, pages.getTotalElements()); } return personRepository.findPerson(pageable); } This isn't a robust solution but was a simple and feasible workaround in my case because we currently only need to support this type of sorting for one specific use case at this time. Also, the repository and code is internal so there isn't currently a need for us to be any more robust. This might not work for everyone running into this problem but was a reasonable workaround for our use case, which we'll use until spring-data-jpa 1.2 is released with this fix.
        Hide
        P. J. Reed added a comment -

        I've found a case in which this is still not fixed. If you have a repository that extends JpaSpecficationExecutor and use the findAll method with a Specification, the resulting query still excludes null matches. I've updated the original reporter's test case to test this against the latest version and verify that it's broken. I'll attach it momentarily...

        Show
        P. J. Reed added a comment - I've found a case in which this is still not fixed. If you have a repository that extends JpaSpecficationExecutor and use the findAll method with a Specification, the resulting query still excludes null matches. I've updated the original reporter's test case to test this against the latest version and verify that it's broken. I'll attach it momentarily...
        Hide
        P. J. Reed added a comment -

        Test case that shows this bug still exists in version 1.2 for repositories that extend JpaSpecificationExecutor.

        Show
        P. J. Reed added a comment - Test case that shows this bug still exists in version 1.2 for repositories that extend JpaSpecificationExecutor.
        Hide
        Oliver Gierke added a comment -

        Is there a chance you open a new ticket for this? This ticket here is about using JPQL defined queries, not about specifications. The fix for this ticket has exactly fixed that. Especially as this fix has already been shipped I don't want to keep the ticket open for a different aspect of the problem.

        Show
        Oliver Gierke added a comment - Is there a chance you open a new ticket for this? This ticket here is about using JPQL defined queries, not about specifications. The fix for this ticket has exactly fixed that. Especially as this fix has already been shipped I don't want to keep the ticket open for a different aspect of the problem.
        Hide
        P. J. Reed added a comment -

        Done; see DATAJPA-277.

        Show
        P. J. Reed added a comment - Done; see DATAJPA-277 .

          People

          • Assignee:
            Oliver Gierke
            Reporter:
            Shelley J. Baker
          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: