HHH-14212 fix Fetch Graph by simply returning false in TwoPhaseLoad#getOverridingEager() when Fetch Graph is being enforced

hibernate-core/src/main/java/org/hibernate/engine/internal/TwoPhaseLoad.java

@@ -204,7 +204,7 @@ public final class TwoPhaseLoad {
 		String entityName = persister.getEntityName();
 		String[] propertyNames = persister.getPropertyNames();
 		final Type[] types = persister.getPropertyTypes();
-		
+
 		for ( int i = 0; i < hydratedState.length; i++ ) {
 			final Object value = hydratedState[i];
 			if ( debugEnabled ) {
@@ -406,7 +406,11 @@ public final class TwoPhaseLoad {
 	}
 
 	/**
-	 * Check if eager of the association is overridden by anything.
+	 * Check if eager of the association is overridden (i.e. skipping metamodel strategy), including (order sensitive):
+	 * <ol>
+	 *     <li>fetch graph</li>
+	 *     <li>fetch profile</li>
+	 * </ol>
 	 *
 	 * @param session session
 	 * @param entityName entity name
@@ -424,6 +428,12 @@ public final class TwoPhaseLoad {
 		// Performance: check type.isCollectionType() first, as type.isAssociationType() is megamorphic
 		if ( associationType.isCollectionType() || associationType.isAssociationType()  ) {
 
+			// we can return false invariably for if the entity has been covered by entity graph,
+			// its associated JOIN has been present in the SQL generated and hence it would be loaded anyway
+			if ( session.isEnforcingFetchGraph() ) {
+				return false;
+			}
+
 			// check 'fetch profile' next; skip 'metamodel' if 'fetch profile' takes effect
 			final Boolean overridingEager = isEagerFetchProfile( session, entityName, associationName );
 

hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java

@@ -522,4 +522,11 @@ public interface SharedSessionContractImplementor
 	 */
 	PersistenceContext getPersistenceContextInternal();
 
+	default boolean isEnforcingFetchGraph() {
+		return false;
+	}
+
+	default void setEnforcingFetchGraph(boolean enforcingFetchGraph) {
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -215,6 +215,8 @@ public class SessionImpl
 
 	private transient TransactionObserver transactionObserver;
 
+	private transient boolean isEnforcingFetchGraph;
+
 	public SessionImpl(SessionFactoryImpl factory, SessionCreationOptions options) {
 		super( factory, options );
 
@@ -3312,6 +3314,10 @@ public class SessionImpl
 				lockOptions = buildLockOptions( lockModeType, properties );
 				loadAccess.with( lockOptions );
 			}
+
+			if ( getLoadQueryInfluencers().getEffectiveEntityGraph().getSemantic() == GraphSemantic.FETCH ) {
+				setEnforcingFetchGraph( true );
+			}
 			
 			return loadAccess.load( (Serializable) primaryKey );
 		}
@@ -3357,6 +3363,7 @@ public class SessionImpl
 		finally {
 			getLoadQueryInfluencers().getEffectiveEntityGraph().clear();
 			getLoadQueryInfluencers().setReadOnly( null );
+			setEnforcingFetchGraph( false );
 		}
 	}
 
@@ -3785,4 +3792,15 @@ public class SessionImpl
 		}
 		return readOnly;
 	}
+
+	@Override
+	public boolean isEnforcingFetchGraph() {
+		return this.isEnforcingFetchGraph;
+	}
+
+	@Override
+	public void setEnforcingFetchGraph(boolean isEnforcingFetchGraph) {
+		this.isEnforcingFetchGraph = isEnforcingFetchGraph;
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/query/internal/AbstractProducedQuery.java

@@ -1437,6 +1437,9 @@ public abstract class AbstractProducedQuery<R> implements QueryImplementor<R> {
 			sessionCacheMode = getProducer().getCacheMode();
 			getProducer().setCacheMode( effectiveCacheMode );
 		}
+		if ( entityGraphQueryHint != null && entityGraphQueryHint.getSemantic() == GraphSemantic.FETCH ) {
+			getProducer().setEnforcingFetchGraph( true );
+		}
 	}
 
 	protected void afterQuery() {
@@ -1448,6 +1451,7 @@ public abstract class AbstractProducedQuery<R> implements QueryImplementor<R> {
 			getProducer().setCacheMode( sessionCacheMode );
 			sessionCacheMode = null;
 		}
+		getProducer().setEnforcingFetchGraph( false );
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/jpa/test/graphs/FetchGraphTest.java

@@ -0,0 +1,176 @@
+package org.hibernate.jpa.test.graphs;
+
+import java.util.Arrays;
+import java.util.List;
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.EntityGraph;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+import javax.persistence.OneToMany;
+import javax.persistence.Table;
+
+import org.hibernate.Hibernate;
+import org.hibernate.annotations.Fetch;
+import org.hibernate.annotations.FetchMode;
+import org.hibernate.graph.GraphParser;
+import org.hibernate.graph.GraphSemantic;
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+
+import org.hibernate.testing.TestForIssue;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Yaroslav Prokipchyn
+ * @author Nathan Xu
+ */
+@TestForIssue( jiraKey = "HHH-14212" )
+public class FetchGraphTest extends BaseEntityManagerFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] {
+				LedgerRecord.class,
+				LedgerRecordItem.class,
+				BudgetRecord.class,
+				Trigger.class,
+				FinanceEntity.class
+		};
+	}
+
+	@Before
+	public void setUp() {
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			Trigger trigger = new Trigger();
+			entityManager.persist( trigger );
+
+			BudgetRecord budgetRecord = new BudgetRecord();
+			budgetRecord.amount = 100;
+			budgetRecord.trigger = trigger;
+			entityManager.persist( budgetRecord );
+
+			FinanceEntity client = new FinanceEntity();
+			client.name = "client";
+			FinanceEntity vendor = new FinanceEntity();
+			vendor.name = "vendor";
+			entityManager.persist( client );
+			entityManager.persist( vendor );
+
+			LedgerRecordItem item1 = new LedgerRecordItem();
+			item1.financeEntity = client;
+			LedgerRecordItem item2 = new LedgerRecordItem();
+			item2.financeEntity = vendor;
+			entityManager.persist( item1 );
+			entityManager.persist( item2 );
+
+			LedgerRecord ledgerRecord = new LedgerRecord();
+			ledgerRecord.budgetRecord = budgetRecord;
+			ledgerRecord.trigger = trigger;
+			ledgerRecord.ledgerRecordItems= Arrays.asList( item1, item2 );
+
+			item1.ledgerRecord = ledgerRecord;
+			item2.ledgerRecord = ledgerRecord;
+
+			entityManager.persist( ledgerRecord );
+		} );
+	}
+
+	@Test
+	public void testCollectionEntityGraph() {
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final EntityGraph<LedgerRecord> entityGraph = GraphParser.parse( LedgerRecord.class, "budgetRecord, ledgerRecordItems.value(financeEntity)", entityManager );
+			final List<LedgerRecord> records = entityManager.createQuery( "from LedgerRecord", LedgerRecord.class )
+					.setHint( GraphSemantic.FETCH.getJpaHintName(), entityGraph )
+					.getResultList();
+			assertThat( records.size(), is( 1 ) );
+			records.forEach( record -> {
+				assertFalse( Hibernate.isInitialized( record.trigger ) );
+				assertTrue( Hibernate.isInitialized( record.budgetRecord ) );
+				assertFalse( Hibernate.isInitialized( record.budgetRecord.trigger ) );
+				assertTrue( Hibernate.isInitialized( record.ledgerRecordItems) );
+				assertThat( record.ledgerRecordItems.size(), is( 2 ) );
+				record.ledgerRecordItems.forEach( item -> {
+					assertSame( record, item.ledgerRecord );
+					assertTrue( Hibernate.isInitialized( item.financeEntity ) );
+				} );
+			} );
+		} );
+	}
+
+	@Entity(name = "LedgerRecord")
+	@Table(name = "LedgerRecord")
+	static class LedgerRecord {
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Integer id;
+
+		@ManyToOne
+		BudgetRecord budgetRecord;
+
+		@OneToMany(mappedBy = "ledgerRecord", fetch = FetchType.EAGER, cascade = CascadeType.ALL)
+		@Fetch(FetchMode.SUBSELECT)
+		List<LedgerRecordItem> ledgerRecordItems;
+
+		@ManyToOne
+		Trigger trigger;
+	}
+
+	@Entity(name = "LedgerRecordItem")
+	@Table(name = "LedgerRecordItem")
+	static class LedgerRecordItem {
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Integer id;
+
+		@ManyToOne
+		LedgerRecord ledgerRecord;
+
+		@ManyToOne
+		FinanceEntity financeEntity;
+	}
+
+	@Entity(name = "BudgetRecord")
+	@Table(name = "BudgetRecord")
+	static class BudgetRecord {
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Integer id;
+
+		int amount;
+
+		@ManyToOne
+		Trigger trigger;
+	}
+
+	@Entity(name = "Trigger")
+	@Table(name = "Trigger")
+	static class Trigger {
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Integer id;
+
+		String name;
+	}
+
+	@Entity(name = "FinanceEntity")
+	@Table(name = "FinanceEntity")
+	static class FinanceEntity {
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Integer id;
+
+		String name;
+	}
+}
+