2

I'm writing a simple Spring Data JPA application. I use MySQL database. There are two simple tables:

  • Department
  • Employee

Each employee works in some department (Employee.department_id).

@Entity
public class Department {
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Id
    private Long id;

    @Basic(fetch = FetchType.LAZY)
    @OneToMany(mappedBy = "department")
    List<Employee> employees;
}

@Entity
public class Employee {
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Id
    private Long id;

    @ManyToOne
    @JoinColumn
    private Department department;
}

@Repository
public interface DepartmentRepository extends JpaRepository<Department, Long> {
    @Query("FROM Department dep JOIN FETCH dep.employees emp WHERE dep = emp.department")
    List<Department> getAll();
}

The method getAll returns a list with duplicated departments (each department is repeated as many times as there are employees in this department).

Question 1: Am I rigth that this is a feature related not to Spring Data JPA, but to to Hibernate?
Question 2: What is the best way to fix it? (I found at least two ways: 1) use Set<Department> getAll(); 2) use "SELECT DISTINCT dep" in @Query annotation)

Daniel
  • 295
  • 2
  • 9

4 Answers4

4

FROM Department dep JOIN FETCH dep.employees emp expression generates native query which returns plain result Department-Employee. Every Department will be returned dep.employees.size() times. This is a JPA-provider expected behavior (Hibernate in your case).

Using distinct to get rid of duplicates seems like a good option. Set<Department> as a query result makes it impossible to get the ordered result.

Oleksii Valuiskyi
  • 2,691
  • 1
  • 8
  • 22
  • Thank you! But Spring is supposed to make things simpler, more convenient. Why does one need to bother about such low-level concepts when working with Spring, OOP? – Daniel Sep 03 '20 at 12:51
  • This is intentional behavior. It allows you to use `emp` as alias for single employee in where clause. For instance, `where emp.name like "%John%"`. – Oleksii Valuiskyi Sep 03 '20 at 13:03
  • Spring give you not only simplicity, but flexibility. – Oleksii Valuiskyi Sep 03 '20 at 13:09
0

You probably want to use LEFT JOIN FETCH as well as DISTINCT. Normal JOIN FETCH uses an inner join so Departments without any employees will not be returned by your query. LEFT JOIN FETCH uses an outer join instead.

-1

I'm not sure about using @Query here. If you mapped these 2 entities via annotations @OneToMany and @ManyToOne this join is redundant. And second moment here, why is @Basic(fetch = FetchType.LAZY) here? Maybe you should set lazy init in @OneToMany ? Check out this resource, I hope you'll find it helpfull

NikMashei
  • 371
  • 1
  • 7
  • 19
-1

With my knowledge, the use case you have mentioned, you don't need to define your own method and instead use repository.findAll which is inherited from PagingAndSortingRepository which extends CrudRepository and implemented by SimpleJpaRepository. See here

So just leave the repository interface blank as below,

@Repository
public interface DepartmentRepository extends JpaRepository<Department, Long> {

}

Then inject and use it wherever you need, e.g. lets say with DepartmentService

public interface DepartmentService {
    
    List<Link> getAll();    
}

@Component
public class DepartmentServiceImpl implements DepartmentService {

    private final DepartmentRepository  repository;    

    // dependency injection via constructor
    @Autowired
    public DepartmentServiceImpl(DepartmentRepository repository) {
        this.repository = repository;
    }

    @Override
    public List<Department> getAll() {  
        return repository.findAll();
    }
}
Sujitmohanty30
  • 3,256
  • 2
  • 5
  • 23
  • As far as I'm aware your approach 1) requires @Transactional to access department.getEmployees() 2) leads to `n+1 select problem` – Daniel Sep 04 '20 at 12:20
  • 1. Why @Transactional required to fetch the data ? 2.`FetchType.LAZY` will take care of the `n+1` problem i believe. Don't know much and if you could elaborate about it may be. My point here to `findAll` you don't need to implement anything and leverage already implemented by JPA. Worth looking at https://stackoverflow.com/questions/27799455/hibernate-creating-n1-queries-for-manytoone-jpa-annotated-property – Sujitmohanty30 Sep 04 '20 at 12:43
  • You can read my previous question https://stackoverflow.com/questions/63675153 – Daniel Sep 04 '20 at 12:50
  • I read it but I feel the problem and solutions are unnecessarily complex where as it should not be. I use Oracle and normally for read only purpose transactional is never required strictly. Please see explanation about @Transactional here https://stackoverflow.com/questions/10394857/how-to-use-transactional-with-spring-data . And also this https://stackoverflow.com/questions/54326306/what-is-the-use-of-transactional-with-jpa-and-hibernate – Sujitmohanty30 Sep 04 '20 at 13:05
  • I can use departmentService.findAll() to get the list of depertments. But then when I call departmens.get(0).getEmployees() an exception is thrown. @Transactional fixes this. – Daniel Sep 04 '20 at 13:17
  • May I know what is the exception ? if you are talking about your previous question then i would suggest to remove the transnational completely and check.. What we do with `JOIN FETCH` can also be done with using `findAll` from repository. Let me know the exception i will check and comeback. – Sujitmohanty30 Sep 04 '20 at 16:30