HHH-12775 - Avoid join on natural id property access

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java

@@ -6,11 +6,6 @@
  */
 package org.hibernate.hql.internal.ast.tree;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Set;
-
 import org.hibernate.QueryException;
 import org.hibernate.engine.internal.JoinSequence;
 import org.hibernate.hql.internal.CollectionProperties;
@@ -21,7 +16,6 @@ import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.log.DeprecationLogger;
 import org.hibernate.internal.util.StringHelper;
-import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.persister.entity.AbstractEntityPersister;
 import org.hibernate.persister.entity.EntityPersister;
@@ -400,7 +394,7 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec
 			// entity's PK (because 'our' table would know the FK).
 			parentAsDotNode = (DotNode) parent;
 			property = parentAsDotNode.propertyName;
-			joinIsNeeded = generateJoin && !isReferenceToPrimaryKey( parentAsDotNode.propertyName, entityType );
+			joinIsNeeded = generateJoin && !isPropertyEmbeddedInJoinProperties( parentAsDotNode.propertyName );
 		}
 		else if ( !getWalker().isSelectStatement() ) {
 			// in non-select queries, the only time we should need to join is if we are in a subquery from clause
@@ -427,7 +421,7 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec
 
 	}
 
-	private boolean isDotNode(AST n) {
+	private static boolean isDotNode(AST n) {
 		return n != null && n.getType() == SqlTokenTypes.DOT;
 	}
 
@@ -583,39 +577,43 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec
 	}
 
 	/**
-	 * Is the given property name a reference to the primary key of the associated
-	 * entity construed by the given entity type?
+	 * Is the given property name a reference to the join key of the associated
+	 * entity constructed by the given entity type?
+	 * <p/>
+	 *
+	 * This method resolves the {@code propertyName} as a property of the entity type at the
+	 * {@link #propertyPath} relative to the {@link #getFromElement() FromElement}.
+	 * The implementation does so by invoking {@link FromElement#getPropertyType(String, String)},
+	 * which will resolve the property path against the entity's {@link org.hibernate.persister.entity.PropertyMapping}.
+	 * On initialization of the {@link EntityPersister}, this {@code PropertyMapping} is filled with
+	 * property paths for all the owned properties and associations, and (embedded) identifier or unique key
+	 * properties for owned associations.
+	 * Henceforth, whenever a property path is found in the {@code PropertyMapping} of the {@code EntityPersister}
+	 * of the {@code FromElement}, we know that the property corresponds to a SQL fragment producible from the
+	 * {@code FromElement}, and as such the entity property can be dereferenced (optimized) in the final query.
+	 *
 	 * <p/>
 	 * For example, consider a fragment like order.customer.id
 	 * (where order is a from-element alias).  Here, we'd have:
 	 * propertyName = "id" AND
-	 * owningType = ManyToOneType(Customer)
-	 * and are being asked to determine whether "customer.id" is a reference
-	 * to customer's PK...
+	 * propertyPath = "customer"
+	 * FromElement = Order
+	 * and are being asked to determine whether "customer.id" is a property path of Order
 	 *
 	 * @param propertyName The name of the property to check.
-	 * @param owningType The type represeting the entity "owning" the property
 	 *
 	 * @return True if propertyName references the entity's (owningType->associatedEntity)
-	 *         primary key; false otherwise.
+	 *         join key; false otherwise.
 	 */
-	private boolean isReferenceToPrimaryKey(String propertyName, EntityType owningType) {
-		EntityPersister persister = getSessionFactoryHelper()
-				.getFactory()
-				.getEntityPersister( owningType.getAssociatedEntityName() );
-		if ( persister.getEntityMetamodel().hasNonIdentifierPropertyNamedId() ) {
-			// only the identifier property field name can be a reference to the associated entity's PK...
-			return propertyName.equals( persister.getIdentifierPropertyName() ) && owningType.isReferenceToPrimaryKey();
+	private boolean isPropertyEmbeddedInJoinProperties(String propertyName) {
+		String propertyPath = String.join( ".", this.propertyPath, propertyName );
+		try {
+			Type propertyType = getFromElement().getPropertyType( this.propertyPath, propertyPath );
+			return propertyType != null;
 		}
-		// here, we have two possibilities:
-		// 1) the property-name matches the explicitly identifier property name
-		// 2) the property-name matches the implicit 'id' property name
-		// the referenced node text is the special 'id'
-		if ( EntityPersister.ENTITY_ID.equals( propertyName ) ) {
-			return owningType.isReferenceToPrimaryKey();
+		catch (QueryException e) {
+			return false;
 		}
-		String keyPropertyName = getSessionFactoryHelper().getIdentifierOrUniqueKeyPropertyName( owningType );
-		return keyPropertyName != null && keyPropertyName.equals( propertyName ) && owningType.isReferenceToPrimaryKey();
 	}
 
 	private void checkForCorrelatedSubquery(String methodName) {
@@ -651,8 +649,9 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec
 			);
 		}
 
-		initText();
 		setPropertyNameAndPath( dotParent ); // Set the unresolved path in this node and the parent.
+		initText();
+
 		// Set the text for the parent.
 		if ( dotParent != null ) {
 			dotParent.dereferenceType = DereferenceType.IDENTIFIER;

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractPropertyMapping.java

@@ -386,7 +386,7 @@ public abstract class AbstractPropertyMapping implements PropertyMapping {
 			}
 		}
 
-		if ( idPropName != null ) {
+		if ( (! etype.isNullable() || etype.isReferenceToPrimaryKey() ) && idPropName != null ) {
 			String idpath2 = extendPath( path, idPropName );
 			addPropertyPath( idpath2, idtype, columns, columnReaders, columnReaderTemplates, null, factory );
 			initPropertyPaths( idpath2, idtype, columns, columnReaders, columnReaderTemplates, null, factory );

hibernate-core/src/main/java/org/hibernate/type/EntityType.java

@@ -659,7 +659,12 @@ public abstract class EntityType extends AbstractType implements AssociationType
 		}
 	}
 
-	protected abstract boolean isNullable();
+	/**
+	 * The nullability of the property.
+	 *
+	 * @return The nullability of the property.
+	 */
+	public abstract boolean isNullable();
 
 	/**
 	 * Resolve an identifier via a load.

hibernate-core/src/main/java/org/hibernate/type/ManyToOneType.java

@@ -109,7 +109,7 @@ public class ManyToOneType extends EntityType {
 	}
 
 	@Override
-	protected boolean isNullable() {
+	public boolean isNullable() {
 		return ignoreNotFound;
 	}
 

hibernate-core/src/main/java/org/hibernate/type/OneToOneType.java

@@ -155,7 +155,7 @@ public class OneToOneType extends EntityType {
 	}
 
 	@Override
-	protected boolean isNullable() {
+	public boolean isNullable() {
 		return foreignKeyType==ForeignKeyDirection.TO_PARENT;
 	}
 

hibernate-core/src/test/java/org/hibernate/test/hql/NaturalIdDereferenceTest.java

@@ -0,0 +1,334 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+
+package org.hibernate.test.hql;
+
+import org.hibernate.annotations.NaturalId;
+import org.hibernate.hql.spi.FilterTranslator;
+import org.hibernate.query.Query;
+
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+import java.util.Collections;
+import java.util.List;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * @author Jan-Willem Gmelig Meyling
+ * @author Christian Beikov
+ */
+public class NaturalIdDereferenceTest extends BaseCoreFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { Book.class, BookRef.class, BookRefRef.class };
+	}
+
+	@Before
+	public void setUp() {
+		doInHibernate( this::sessionFactory, session -> {
+			Book book = new Book();
+			book.isbn = "abcd";
+			session.persist( book );
+
+			BookRef bookRef = new BookRef();
+			bookRef.naturalBook = bookRef.normalBook = book;
+			session.persist( bookRef );
+
+			session.flush();
+			session.clear();
+		} );
+	}
+
+	@Test
+	public void naturalIdDereferenceTest() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r.normalBook.isbn FROM BookRef r" );
+			List resultList = query.getResultList();
+			assertFalse( resultList.isEmpty() );
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	@Test
+	public void normalIdDereferenceFromAlias() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r.normalBook.id FROM BookRef r" );
+			List resultList = query.getResultList();
+			assertFalse( resultList.isEmpty() );
+			assertEquals( 0, getSQLJoinCount( query ) );
+		} );
+	}
+
+
+	@Test
+	public void naturalIdDereferenceFromAlias() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r.naturalBook.isbn FROM BookRef r" );
+			List resultList = query.getResultList();
+			assertFalse( resultList.isEmpty() );
+			assertEquals( 0, getSQLJoinCount( query ) );
+		} );
+	}
+
+	@Test
+	public void normalIdDereferenceFromImplicitJoin() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.normalBookRef.normalBook.id FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	@Test
+	public void naturalIdDereferenceFromImplicitJoin() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.normalBookRef.naturalBook.isbn FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * Due to the construction of the mapping for {@link BookRefRef#naturalBookRef}, the {@code isbn} column maps
+	 * to both the referenced {@link BookRef} and {@link Book}. As such, {@code r2.naturalBookRef.naturalBook.isbn}
+	 * can be dereferenced without a single join.
+	 */
+	@Test
+	public void nestedNaturalIdDereferenceFromImplicitJoin() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.naturalBookRef.naturalBook.isbn FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 0, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * Adjustment of {@link #nestedNaturalIdDereferenceFromImplicitJoin()}, that instead selects the {@code id} property,
+	 * which requires a single join to {@code Book}.
+	 */
+	@Test
+	public void nestedNaturalIdDereferenceFromImplicitJoin2() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.naturalBookRef.naturalBook.id FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	@Test
+	public void doNotDereferenceNaturalIdIfIsReferenceToPrimaryKey() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.normalBookRef.normalBook.isbn FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 2, getSQLJoinCount( query ) );
+		} );
+	}
+
+	@Test
+	public void selectedEntityIsNotDereferencedForPrimaryKey() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.normalBookRef.normalBook FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 2, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * BookRefRef can be joined directly with Book due to the construction of the isbn key.
+	 * <p>
+	 * I.e.
+	 * <p>
+	 * BookRefRef{isbn=abcd} enforces BookRef{isbn=abc} (FK) enforces BookRef{isbn=abc} (FK),
+	 * so bookRefRef.naturalBookRef.naturalBook = Book{isbn=abc}.
+	 * <p>
+	 * BookRefRef{isbn=null}, i.e. no BookRef for this BookRefRef, and as such no book,
+	 * so bookRefRef.naturalBookRef.naturalBook yields null which is expected.
+	 */
+	@Test
+	public void selectedEntityIsNotDereferencedForNaturalId() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT r2.naturalBookRef.naturalBook FROM BookRefRef r2" );
+			query.getResultList();
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * {@code r2.normalBookRef.normalBook.id} requires 1 join as seen in {@link #normalIdDereferenceFromImplicitJoin}.
+	 * {@code r3.naturalBookRef.naturalBook.isbn} requires 1 join as seen in {@link #selectedEntityIsNotDereferencedForNaturalId()}.
+	 * An additional join is added to join BookRef once more on {@code r2.normalBookRef.normalBook.isbn = r3.naturalBookRef.naturalBook.isbn}.
+	 * This results in three joins in total.
+	 */
+	@Test
+	public void dereferenceNaturalIdInJoin() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery(
+					"SELECT r2.normalBookRef.normalBook.id, r3.naturalBookRef.naturalBook.isbn " +
+							"FROM BookRefRef r2 JOIN BookRefRef r3 ON r2.normalBookRef.normalBook.isbn = r3.naturalBookRef.naturalBook.isbn" );
+			query.getResultList();
+			// r2.normalBookRef.normalBook.id requires
+			assertEquals( 3, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * {@code BookRefRef} is joined with {@code BookRef} on {@code b.naturalBook.isbn = a.naturalBookRef.naturalBook.isbn}.
+	 * {@code b.naturalBook.isbn} can be dereferenced without any join ({@link #naturalIdDereferenceFromAlias()}).
+	 * {@code a.naturalBookRef.naturalBook.isbn} can be dereferenced without any join ({@link #nestedNaturalIdDereferenceFromImplicitJoin()}).
+	 * We finally select all properties of {@code b.normalBook}, which requires {@code Book} to be joined.
+	 * This results in two joins in total.
+	 */
+	@Test
+	public void dereferenceNaturalIdInJoin2() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery( "SELECT b.normalBook FROM BookRefRef a " +
+													   "JOIN BookRef b ON b.naturalBook.isbn = a.naturalBookRef.naturalBook.isbn" );
+			query.getResultList();
+			assertEquals( 2, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * The {@link BookRef#normalBook} is joined with {@code BookRef} on {@code join.isbn = from.normalBook.isbn}.
+	 * {@code join.isbn = from.normalBook.isbn} both dereference to {@code join.isbn}.
+	 * {@code r.normalBook.isbn} dereferences to {@code join.isbn}.
+	 * As a result, only a single join is required.
+	 */
+	@Test
+	public void dereferenceNaturalIdInJoin3() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery(
+					"SELECT r.normalBook.isbn FROM BookRef r JOIN r.normalBook b ON b.isbn = r.normalBook.isbn" );
+			query.getResultList();
+			assertEquals( 1, getSQLJoinCount( query ) );
+		} );
+	}
+
+	/**
+	 * The {@link Book} is joined with {@link BookRef} on {@code book.isbn = ref.normalBook.isbn}.
+	 * {@code book.isbn} can be dereferenced from the {@code Book} table.
+	 * {@code ref.normalBook.isbn} requires an implicit join with book.
+	 * {@code ref.normalBook.isbn} in the final selection is available due to the aforementioned join.
+	 * As a result, 2 joins are required.
+	 */
+	@Test
+	public void dereferenceNaturalIdInJoin4() {
+		doInHibernate( this::sessionFactory, session -> {
+			Query query = session.createQuery(
+					"SELECT r.normalBook.isbn FROM BookRef r JOIN Book b ON b.isbn = r.normalBook.isbn" );
+			query.getResultList();
+			assertEquals( 2, getSQLJoinCount( query ) );
+		} );
+	}
+
+	private int getSQLJoinCount(Query query) {
+		String sqlQuery = getSQLQuery( query ).toLowerCase();
+
+		int lastIndex = 0;
+		int count = 0;
+
+		while ( lastIndex != -1 ) {
+
+			lastIndex = sqlQuery.indexOf( " join ", lastIndex );
+
+			if ( lastIndex != -1 ) {
+				count++;
+				lastIndex += " join ".length();
+			}
+		}
+
+		return count;
+	}
+
+	private String getSQLQuery(Query query) {
+		FilterTranslator naturalIdJoinGenerationTest1 = this.sessionFactory()
+				.getSettings()
+				.getQueryTranslatorFactory()
+				.createFilterTranslator(
+						"nid",
+						query.getQueryString(),
+						Collections.emptyMap(),
+						this.sessionFactory()
+				);
+		naturalIdJoinGenerationTest1.compile( Collections.emptyMap(), false );
+		return naturalIdJoinGenerationTest1.getSQLString();
+	}
+
+	@Override
+	protected boolean isCleanupTestDataRequired() {
+		return true;
+	}
+
+	@Override
+	protected boolean isCleanupTestDataUsingBulkDelete() {
+		return true;
+	}
+
+	@Entity(name = "Book")
+	@Table(name = "book")
+	public static class Book {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.AUTO)
+		private Long id;
+
+		@NaturalId
+		@Column(name = "isbn", unique = true)
+		private String isbn;
+
+	}
+
+	@Entity(name = "BookRef")
+	@Table(name = "bookref")
+	public static class BookRef {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.AUTO)
+		private Long id;
+
+		@ManyToOne(optional = true)
+		@JoinColumn(nullable = true, columnDefinition = "id_ref")
+		private Book normalBook;
+
+		@ManyToOne
+		@JoinColumn(name = "isbn_ref", referencedColumnName = "isbn")
+		private Book naturalBook;
+
+	}
+
+	@Entity(name = "BookRefRef")
+	@Table(name = "bookrefref")
+	public static class BookRefRef {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.AUTO)
+		private Long id;
+
+		@ManyToOne
+		@JoinColumn(nullable = true, columnDefinition = "id_ref_ref")
+		private BookRef normalBookRef;
+
+		@ManyToOne
+		@JoinColumn(name = "isbn_ref_Ref", referencedColumnName = "isbn_ref")
+		private BookRef naturalBookRef;
+	}
+
+}