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

Do not rely on ManagedType by default #3275

Closed
wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Dec 18, 2023

ManagedType may be erased type if the attribute is declared as generic, take Hibernate 6.x for example, exception is thrown like:

java.lang.IllegalArgumentException: Unable to locate Attribute with the given name [name] on this ManagedType [java.lang.Object]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.checkNotNull(AbstractManagedType.java:225) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:148) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:43) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.springframework.data.jpa.repository.query.QueryUtils.requiresOuterJoin(QueryUtils.java:836) ~[spring-data-jpa-3.2.0.jar:3.2.0]

Fix GH-3274

ManagedType may be erased type if the attribute is declared as generic, take Hibernate 6.x for example, exception is thrown like:
```
java.lang.IllegalArgumentException: Unable to locate Attribute with the given name [name] on this ManagedType [java.lang.Object]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.checkNotNull(AbstractManagedType.java:225) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:148) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:43) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.springframework.data.jpa.repository.query.QueryUtils.requiresOuterJoin(QueryUtils.java:836) ~[spring-data-jpa-3.2.0.jar:3.2.0]
```

Fix spring-projectsGH-3274
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 18, 2023
Comment on lines +825 to +846
if (model.getClass().getName().startsWith("org.eclipse.persistence")) {
// required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
// regardless of which join operation is specified next
// see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
// still occurs as of 2.7
ManagedType<?> managedType = null;
if (model instanceof ManagedType) {
managedType = (ManagedType<?>) model;
} else if (model instanceof SingularAttribute
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
}
if (managedType != null) {
propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
} else {
propertyPathModel = from.get(segment).getModel();
}
} else {
// ManagedType may be erased type for some vendor if the attribute is declared as generic
// see: https://hibernate.atlassian.net/browse/HHH-16144
// see: https://github.com/hibernate/hibernate-orm/pull/7630
// see: https://github.com/jakartaee/persistence/issues/562
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if we could hide the vendor specifics behind eg. the PersistenceProvider so we can encapsulate different behaviours when getting hold of the attribute .

I'm thinking of something that would allow us to do something like the following:

Bindable<?> propertyPathModel = PersistenceProvider.segmentResolver(from).resolve(segment);

Having PersistenceProvider implement a let's call it PathSegmentResolver defaulting to from.get(segment).getModel(), we could move the eclipselink specific requirements with all the type checks to the ECLIPSELINK implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to resolve PersistenceProvider in method QueryUtils::requiresOuterJoin?

// ManagedType may be erased type for some vendor if the attribute is declared as generic
// see: https://hibernate.atlassian.net/browse/HHH-16144
// see: https://github.com/hibernate/hibernate-orm/pull/7630
// see: https://github.com/jakartaee/persistence/issues/562
propertyPathModel = from.get(segment).getModel();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from.get("parent").getModel() returns the erased type instead of the correct one inferred from type parameters.

@mp911de
Copy link
Member

mp911de commented Jan 10, 2024

This isn't a working fix, it just adds some additional complexity that happens to work by accident and not by intent.

@mp911de mp911de closed this Jan 10, 2024
@mp911de mp911de added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic @MappedSuperClass class doesn't works since 3.0
4 participants