persistence
persistence copied to clipboard
@ManyToOne defaults to eager fetching and this is almost always wrong
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:
-
Introduce a new member to
FetchType
:public enum FetchType { LAZY, EAGER, DEFAULT }
-
Change the definition of
@ManyToOne
topublic @interface ManyToOne { ... FetchType fetch() default DEFAULT; }
(And do the same to
@OneToOne
.) -
Introduce a configuration property
jakarta.persistence.defaultFetchType
which acceptsEAGER
orLAZY
. Specify that it defaults toEAGER
.
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?
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?
@gastaldi not sure what you mean ... example?
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.
@gastaldi well according to what I proposed, that code would not be affected for three reasons:
- it's already broken in JPA, since
@OneToMany
defaults toLAZY
, - anyway, the proposal doesn't affect the fetching of
@OneToMany
associations at all, since they remainfetch=LAZY
by default, and -
jakarta.persistence.defaultFetchType
would default toEAGER
, 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
.
Shouldn't the association be part of the property name then? Eg. jakarta.persistence.manyToOne.defaultFetchType=LAZY
?
Yes, probably.
@gavinking if this change doesn't happen, what is preventing hibernate to have such config option?
@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.
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
@AleksNo In the proposal above, there is no change to behavior unless you explicitly set jakarta.persistence.defaultFetchType=LAZY
.
#455 shows what this could look like.
@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.
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.