@ManyToOne defaults to eager fetching and this is almost always wrong

[gavinking]

Currently we have:

public @interface ManyToOne {
    ...
    FetchType fetch() default FetchType.EAGER;
}

This is actually a really bad default, and has a tendency to lead to N+1 selects. And of course it's also inconsistent with the default for @OneToMany and @ManyToMany.

I propose that we finally fix this. I've been trying to see how we could do that without breaking existing programs, and here's what I've come up with:

1. Introduce a new member to FetchType:

   public enum FetchType { LAZY, EAGER, DEFAULT }
   

2. Change the definition of @ManyToOne to

   public @interface ManyToOne {
       ...
       FetchType fetch() default DEFAULT;
   }
   

   (And do the same to @OneToOne.)

3. Introduce a configuration property jakarta.persistence.defaultFetchType which accepts EAGER or LAZY. Specify that it defaults to EAGER.

One might object that this proposal looks like it breaks binary compatibility, but I would argue that it is effectively binary back-compatible, in that programs compiled against the older APIs continue to work.

WDYT?

[gastaldi]

So that fetch type could be overriden globally regardless if used in a @ManyToOne, @OneToMany, @ManyToMany , @OneToOne or @Basic? Wouldn't that mess with some queries that expect the default fetch type for each relationship?

[gavinking]

@gastaldi not sure what you mean ... example?

[gastaldi]

My concern is that introducing a property that impacts every association fetch type may introduce unexpected behavior, specially when your code queries a relationship assuming the default fetch type, like the Book x Author example:

EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
 
TypedQuery<Author> q = em.createQuery(
        "SELECT a FROM Author a",
        Author.class);
List<Author> authors = q.getResultList();
em.getTransaction().commit();
em.close();
 
for (Author author : authors) {
    List<Book> books = author.getBooks();
    log.info("... the next line should throw LazyInitializationException but it may not if the default fetch type is overridden");
    books.size();
}

Maybe it's not a legitimate concern but I think overriding this property may break existing legacy working code.

[gavinking]

@gastaldi well according to what I proposed, that code would not be affected for three reasons:

1. it's already broken in JPA, since @OneToMany defaults to LAZY,
2. anyway, the proposal doesn't affect the fetching of @OneToMany associations at all, since they remain fetch=LAZY by default, and
3. jakarta.persistence.defaultFetchType would default to EAGER, i.e. no change of behavior of @ManyToOne either.

What this proposal allows is for the user to explicitly set:

 jakarta.persistence.defaultFetchType=LAZY

instead of having to go set fetch=LAZY on every single @ManyToOne.

[gastaldi]

Shouldn't the association be part of the property name then? Eg. jakarta.persistence.manyToOne.defaultFetchType=LAZY?

[gavinking]

Yes, probably.

[xDeyan]

@gavinking if this change doesn't happen, what is preventing hibernate to have such config option?

[gavinking]

@xDeyan indeed, it might be possible to add this to Hibernate, since using Jandex we can apparently distinguish an annotation that's explicitly set from an annotation that's defaulted.

But I would much rather fix the spec than have to add another option to deviate from the spec.

[AleksNo]

Hi,

the problem with this change is, it could break the backwards compatibility in some cases. Working with detached entities is such a case if the code relies on it, that a @ManyToOne-mapping is always FetchType.EAGER as the default. So i would not implement this change too fast. IMHO the developers should get some time to check their codebase and fix it if necessary.

Cheers

[gavinking]

@AleksNo In the proposal above, there is no change to behavior unless you explicitly set jakarta.persistence.defaultFetchType=LAZY.

[gavinking]

#455 shows what this could look like.

[gavinking]

@DefaultFetchType(LAZY) could also be a package-level annotation. This makes a lot of sense because it limits the scope of the setting, enabling incremental migration.

[t-beckmann]

The …ToOne fetch can always be joined into the statement selecting the entity itself. No N+1 in the …ToOne case. The eager fetch here actually saves additional database roundtrips. It is a sensible default in …ToOne relations.