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

Specifications with sort creates additional join even though the entity was already fetched #2253

Open
michalkrajcovic opened this issue Jul 9, 2021 · 8 comments · May be fixed by #2434
Open
Assignees
Labels
status: blocked An issue that's blocked on an external project change

Comments

@michalkrajcovic
Copy link

Hi, specifications with sort creates additional join even though the entity was already fetched. The result is that the table is then left joined twice. Consider setup:
entities

@Entity
public class A {
    @Id
    Long id

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "b_id")
    B b;
}

@Entity
public class B {
    @Id
    Long id
    String name
}

specifications

Specification<A> specification =
(root, query, builder) -> {
   query.distinct(true); 
   root.fetch(A_.b, JoinType.LEFT);
   return builder.equal(root.get(A_.id), 1L);
};

Sort sort = Sort.by(Sort.Direction.ASC, "b.name");

aRepository.findAll(specification, sort);

then
List<T> findAll(Specification<T> spec, Sort sort);

creates SQL query

select distinct A1.id, A1.b_id, B2.id, B2.name
from A A1
    left join B B1 on A1.b_id = B1.id
    left join B B2 on A1.b_id = B2.id
where A.id = 1
order by B1.name asc

B entity is joined twice. If the query is distinct, then it fails with SQL error
Expression #1 of ORDER BY clause is not in SELECT list, references column ... B1.name ... which is not in SELECT list; this is incompatible with DISTINCT
The SQL error in this case is correct, order by clause is not in select list. The sort should not create additional join if the entity was already fetched or joined.

The commit that introduced this behaviour is 43d1438

Previously, it checked whether there already is a fetched entity (isAlreadyFetched() method) if yes then it didn’t create additional join (getOrCreateJoin() method). The logic was changed to check whether the entity was already inner-joined. But even if I change the join type in specification to JoinType.INNER (root.fetch(A_.b, JoinType.INNER);) it still creates additional join.

Possible duplicate of #2206

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 9, 2021
@schauder schauder added the type: regression A regression from a previous release label Jul 12, 2021
schauder added a commit that referenced this issue Jul 13, 2021
The test is green.
The problem seems to be that root has a "fetch" and a "join" which both end up being translated to a `JOIN` when converted to SQL.

See #2253
@schauder
Copy link
Contributor

schauder commented Jul 14, 2021

This behaviour is actually correct.
The real problem is that a Specification is really a (leaky) abstraction over a WHERE-clause.
By using fetch and distinct you are not only affecting the WHERE-clause though, but also the projection, i.e. the SELECT clause.
This leads to confusion and ultimately errors.

The problem with a JOIN FETCH is, it isn't really a join. From.getJoins() doesn't return it, which makes sense, since it must not be referenced anywhere in the rest of the statement.
From the Java Persistence API, Version 2.2 Section 4.4.5.3 Fetch Joins

A FETCH JOIN enables the fetching of an association or element collection as a side effect of the exe- cution of a query.
The syntax for a fetch join is
fetch_join ::= [ LEFT [OUTER] | INNER ] JOIN FETCH join_association_path_expression (emphasis is mine)
The association referenced by the right side of the FETCH JOIN clause must be an association or element collection that is referenced from an entity or embeddable that is returned as a result of the query. It is not permitted to specify an identification variable for the objects referenced by the right side of the FETCH JOIN clause, and hence references to the implicitly fetched entities or elements cannot appear elsewhere in the query.

This talks about JPQL but I think it is save to assume the equivalent holds true for the Criteria-API used by a Specification.

This means the fetch-join must not and can not be used in a WHERE-clause or ORDER-BY-clause.
Therefore Spring Data JPA has to create an additional JOIN for that purpose.

This of course clashes on the SQL level with the DISTINCT since it basically ignores the additional JOIN and therefore it can't be used in the ORDER-BY-clause.

There is a good Stack Overflow answer regarding this topic.

@schauder schauder added status: declined A suggestion or change that we don't feel we should currently apply status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged type: regression A regression from a previous release labels Jul 14, 2021
@michalkrajcovic
Copy link
Author

@schauder considering the entities described in this issue, if I create a specification

Specification<A> specification =
(root, query, builder) -> {
   query.distinct(true); 
   root.fetch(A_.b, JoinType.LEFT);
   return builder.equal(root.get(A_.b).get(B_.name), “smth”)

};

aRepository.findAll(specification);

please note the equal predicate

then

List<T> findAll(Specification<T> spec);

then creates SQL

select distinct A1.id, A1.b_id, B1.id, B1.name
from A A1
   left join B B1 on A1.b_id = B1.id
where B1.name = "smth"

fetch-join is used in the WHERE clause. No additional join is created.

based on your statement

This means the fetch-join must not and can not be used in a WHERE-clause or ORDER-BY-clause.
Therefore Spring Data JDBC has to create an additional JOIN for that purpose.

Are you sayin that observed behaviour is a bug and additional join should have been created?

@schauder
Copy link
Contributor

Are you sayin that observed behaviour is a bug and additional join should have been created?

According to my understanding of the specification this is indeed wrong behaviour.
It results in an A being loaded without the B.
I might compromise on "inconsistent behaviour" since if the join is reused by a predicate it should be returned by From.getJoins().

I reopen the issue.
I still think it is not us to fix it, but we should create a proper JPA based reproducer so we can ask the JPA maintainers and implementors for their opinion.

@schauder schauder reopened this Jul 15, 2021
@schauder
Copy link
Contributor

I created a sample project and an issue with the JPA Spec in order to get clarification.

@schauder schauder added status: blocked An issue that's blocked on an external project change and removed status: declined A suggestion or change that we don't feel we should currently apply status: invalid An issue that we don't feel is valid labels Jul 19, 2021
AntonMolganov added a commit to AntonMolganov/spring-data-jpa that referenced this issue Feb 8, 2022
AntonMolganov added a commit to AntonMolganov/spring-data-jpa that referenced this issue Feb 8, 2022
AntonMolganov added a commit to AntonMolganov/spring-data-jpa that referenced this issue Feb 8, 2022
AntonMolganov added a commit to AntonMolganov/spring-data-jpa that referenced this issue Apr 14, 2022
@landsman
Copy link

Thanks for the issue.
This would help us a lot currently 🙏

@bean-ben-heb
Copy link

+1
trying to use distinct with join fetch and order by a field in the join but the order by is wrong it that it isn't the table that was fetched but instead a duplicate join that's not in the select
Here is what is produced

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
   left join a.astat a2_0 on a2_0.stat_cd=c1_0.stat_cd
order by a2_0.seq_no offset 0 rows fetch first 10 rows only;

but i want

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
order by a1_0.seq_no offset 0 rows fetch first 10 rows only;

@ilagunas-aoc
Copy link

+1 trying to use distinct with join fetch and order by a field in the join but the order by is wrong it that it isn't the table that was fetched but instead a duplicate join that's not in the select Here is what is produced

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
   left join a.astat a2_0 on a2_0.stat_cd=c1_0.stat_cd
order by a2_0.seq_no offset 0 rows fetch first 10 rows only;

but i want

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
order by a1_0.seq_no offset 0 rows fetch first 10 rows only;

Did you find any solution to this?

@bean-ben-heb
Copy link

+1 trying to use distinct with join fetch and order by a field in the join but the order by is wrong it that it isn't the table that was fetched but instead a duplicate join that's not in the select Here is what is produced

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
   left join a.astat a2_0 on a2_0.stat_cd=c1_0.stat_cd
order by a2_0.seq_no offset 0 rows fetch first 10 rows only;

but i want

select distinct c1_0.c_id,
a1_0.stat_cd,
a1_0.stat_desc,
a1_0.seq_no,
c1_0.stat_cd,
c1_0.c_attr,
from a.ctable c1_0
   left join a.astat a1_0 on a1_0.stat_cd=c1_0.stat_cd
order by a1_0.seq_no offset 0 rows fetch first 10 rows only;

Did you find any solution to this?

I had to right my own specification for this whole thing and override my Spring JPA repository handling of the query
without much context, here is the method i wrote

public static <T> Specification<T> distinctFetchJoinOrder(String fetchAttribute, String joinAttribute, Pageable pageable) {
        return (root, query, criteriaBuilder) -> {
            query.distinct(true);
            var join = (Join<?, ?>) root.fetch(fetchAttribute, JoinType.LEFT);
            List<Order> orders = new ArrayList<>();
            orders.add(criteriaBuilder.asc(join.get(joinAttribute)));
            pageable.getSort().stream().map(order -> {
                if (order.isAscending()) {
                    return criteriaBuilder.asc(root.get(order.getProperty()));
                } else {
                    return criteriaBuilder.desc(root.get(order.getProperty()));
                }
            }).forEach(orders::add);
            query.orderBy(orders);
            return null;
        };
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: blocked An issue that's blocked on an external project change
Projects
None yet
7 participants