Remove unnecessary join when filtering on relationship id

[bukajsytlos]

Closes #3349

[bukajsytlos]

propertyPathModel being null is no longer issue in hibernate

[mp911de]

Thanks a lot. Does this optimization also work for composite id's and/or parts of a composite identifier?

[bukajsytlos]

It should. I've added some more tests. Unfortunately findByRelationshipPartialIdClass is failing on main branch, but is working on 3.4.x (both hibernate 6.x or 7.x).
To me it looks like hibernate issue, but if you could take a look it would be nice

org.springframework.orm.jpa.JpaSystemException: Error accessing field [private org.springframework.data.jpa.domain.sample.IdClassExampleDepartment org.springframework.data.jpa.domain.sample.IdClassExampleEmployee.department] by reflection for persistent property [org.springframework.data.jpa.domain.sample.IdClassExampleEmployee#department] : org.springframework.data.jpa.domain.sample.IdClassExampleEmployeePK@3e1

Test also fails without my changes to query utils

[mp911de]

Looks like a Hibernate issue. The query is:

SELECT r FROM org.springframework.data.jpa.domain.sample.ReferencingIdClassExampleEmployee r LEFT JOIN r.employee e WHERE e.empId = :employeeId

Exception (it's a masked ClassCastException):

java.lang.ClassCastException: Cannot cast org.springframework.data.jpa.domain.sample.IdClassExampleEmployeePK to org.springframework.data.jpa.domain.sample.IdClassExampleEmployee
	at java.base/java.lang.Class.cast(Class.java:3523)
	at java.base/jdk.internal.reflect.MethodHandleObjectFieldAccessorImpl.get(MethodHandleObjectFieldAccessorImpl.java:57)
	at java.base/java.lang.reflect.Field.get(Field.java:440)
	at org.hibernate.property.access.spi.GetterFieldImpl.get(GetterFieldImpl.java:49)
	at org.hibernate.metamodel.mapping.internal.AbstractEmbeddableMapping.getValue(AbstractEmbeddableMapping.java:111)
	at org.hibernate.sql.results.graph.embeddable.internal.NonAggregatedIdentifierMappingInitializer.resolveInstanceSubInitializers(NonAggregatedIdentifierMappingInitializer.java:321)
	at org.hibernate.sql.results.graph.embeddable.internal.NonAggregatedIdentifierMappingInitializer.resolveInstance(NonAggregatedIdentifierMappingInitializer.java:308)
	at org.hibernate.sql.results.graph.embeddable.internal.NonAggregatedIdentifierMappingInitializer.resolveInstance(NonAggregatedIdentifierMappingInitializer.java:45)
	at org.hibernate.sql.results.graph.Initializer.resolveInstance(Initializer.java:151)
	at org.hibernate.sql.results.graph.entity.internal.EntitySelectFetchInitializer.resolveInstance(EntitySelectFetchInitializer.java:186)
	at org.hibernate.sql.results.graph.entity.internal.EntitySelectFetchInitializer.resolveInstance(EntitySelectFetchInitializer.java:42)
	at org.hibernate.sql.results.graph.Initializer.resolveInstance(Initializer.java:151)
	at org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.resolveInstanceSubInitializers(EntityInitializerImpl.java:679)
	at org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.resolveKey(EntityInitializerImpl.java:606)
	at org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.resolveKey(EntityInitializerImpl.java:469)
	at org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.resolveKey(EntityInitializerImpl.java:95)
	at org.hibernate.sql.results.internal.StandardRowReader.coordinateInitializers(StandardRowReader.java:230)
	at org.hibernate.sql.results.internal.StandardRowReader.readRow(StandardRowReader.java:136)
	at org.hibernate.sql.results.spi.ListResultsConsumer.read(ListResultsConsumer.java:245)
	at org.hibernate.sql.results.spi.ListResultsConsumer.readRows(ListResultsConsumer.java:235)
	at org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:166)
	at org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:33)
	at org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.doExecuteQuery(JdbcSelectExecutorStandardImpl.java:211)
	at org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.executeQuery(JdbcSelectExecutorStandardImpl.java:106)
	at org.hibernate.sql.exec.spi.JdbcSelectExecutor.executeQuery(JdbcSelectExecutor.java:64)
	at org.hibernate.sql.exec.spi.JdbcSelectExecutor.list(JdbcSelectExecutor.java:138)
	at org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.lambda$new$1(ConcreteSqmSelectQueryPlan.java:145)
	at org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.withCacheableSqmInterpretation(ConcreteSqmSelectQueryPlan.java:456)
	at org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.performList(ConcreteSqmSelectQueryPlan.java:389)
	at org.hibernate.query.sqm.internal.QuerySqmImpl.doList(QuerySqmImpl.java:401)
	at org.hibernate.query.spi.AbstractSelectionQuery.list(AbstractSelectionQuery.java:144)
	at org.hibernate.query.Query.getResultList(Query.java:121)
	at org.springframework.data.jpa.repository.query.JpaQueryExecution$CollectionExecution.doExecute(JpaQueryExecution.java:130)
	at org.springframework.data.jpa.repository.query.JpaQueryExecution.execute(JpaQueryExecution.java:97)
	at org.springframework.data.jpa.repository.query.AbstractJpaQuery.doExecute(AbstractJpaQuery.java:164)
	at org.springframework.data.jpa.repository.query.AbstractJpaQuery.execute(AbstractJpaQuery.java:154)
	at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:169)
	at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:158)
	at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:167)
	at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:146)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:69)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:369)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:118)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:135)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:138)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:222)
	at jdk.proxy2/jdk.proxy2.$Proxy168.findByEmployee_EmpId(Unknown Source)
	at org.springframework.data.jpa.repository.RepositoryWithCompositeKeyTests.findByRelationshipPartialIdClass(RepositoryWithCompositeKeyTests.java:439)

I've reproduced the issue just using EntityManager. Interestingly, the issue only surfaces when running the query in the same transaction, outside of the transaction, the issue no longer seems to happen. Also, the reproducer catches a different exception, however, it shows the same assignment mismatch. You can find the reproducer and exception message at https://gist.github.com/mp911de/33517cdeedc4fabcc1a6b807976396db.

Also, noteworthy, when introducing equals/hashCode to all types (IdClassExampleEmployee, ReferencingIdClassExampleEmployee) being asserted and flushing/clearing EntityManager, then your test passes.

[bukajsytlos]

I am not sure from the last comment whether there is something that I should change to move this PR forward?

[mp911de]

If you want, please report the problem to the Hibernate team using code from the reproducer. Hibernate folks are somewhat picky and want folks to use their reproducer templates. In recent reports, the moment the've seen Spring being involved, they backed off immediately and so I am tired filing bug reports there. Rather, I focus on investigating the cause to see whether there is something we can fix on our side (which we can't).

Circling back to the original request, I suggest to wait for a Hibernate fix before we proceed. Shipping a feature where composite Id's cannot be used feels like an incomplete feature from our side.

[bukajsytlos]

Ok I will try to report it using their template, although I feel what you are saying.
Regardless, as I said, this test is failing even without my changes, so I don't see it as blocker.

[mp911de]

Technically, it isn't a blocker, but once we're going to advertise the feature it receives more attention and specific cases are going to fail creating a hard time for users wanting to use the feature. Let's get some indication from the Hibernate team how it goes to get a better understanding of how and when we can progress.

[bukajsytlos]

https://hibernate.atlassian.net/browse/HHH-19625