HHH-8879 HHH-8994 : Embeddable with associations used as a Map key

hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/returns/AbstractCollectionReference.java

@@ -23,6 +23,7 @@
  */
 package org.hibernate.loader.plan.build.internal.returns;
 
+import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.loader.PropertyPath;
 import org.hibernate.loader.plan.build.internal.spaces.CompositePropertyMapping;
 import org.hibernate.loader.plan.build.internal.spaces.QuerySpaceHelper;
@@ -34,6 +35,7 @@ import org.hibernate.loader.plan.spi.CollectionFetchableIndex;
 import org.hibernate.loader.plan.spi.CollectionReference;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.collection.CollectionPropertyNames;
+import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.persister.entity.PropertyMapping;
 import org.hibernate.type.CompositeType;
@@ -50,6 +52,9 @@ public abstract class AbstractCollectionReference implements CollectionReference
 	private final CollectionFetchableIndex index;
 	private final CollectionFetchableElement element;
 
+	private final boolean allowElementJoin;
+	private final boolean allowIndexJoin;
+
 	protected AbstractCollectionReference(
 			ExpandingCollectionQuerySpace collectionQuerySpace,
 			PropertyPath propertyPath,
@@ -57,13 +62,31 @@ public abstract class AbstractCollectionReference implements CollectionReference
 		this.collectionQuerySpace = collectionQuerySpace;
 		this.propertyPath = propertyPath;
 
-		this.index = buildIndexGraph( collectionQuerySpace, shouldIncludeJoins );
-		this.element = buildElementGraph( collectionQuerySpace, shouldIncludeJoins );
+		this.allowElementJoin = shouldIncludeJoins;
+
+		// Currently we can only allow a join for the collection index if all of the following are true:
+		// - collection element joins are allowed;
+		// - index is an EntityType;
+		// - index values are not "formulas" (e.g., a @MapKey index is translated into "formula" value(s)).
+		// Hibernate cannot currently support eager joining of associations within a component (@Embeddable) as an index.
+		if ( shouldIncludeJoins &&
+				collectionQuerySpace.getCollectionPersister().hasIndex() &&
+				collectionQuerySpace.getCollectionPersister().getIndexType().isEntityType()  ) {
+			final String[] indexFormulas =
+					( (QueryableCollection) collectionQuerySpace.getCollectionPersister() ).getIndexFormulas();
+			final int nNonNullFormulas = ArrayHelper.countNonNull( indexFormulas );
+			this.allowIndexJoin = nNonNullFormulas == 0;
+		}
+		else {
+			this.allowIndexJoin = false;
+		}
+
+		// All other fields must be initialized before building this.index and this.element.
+		this.index = buildIndexGraph();
+		this.element = buildElementGraph();
 	}
 
-	private CollectionFetchableIndex buildIndexGraph(
-			ExpandingCollectionQuerySpace collectionQuerySpace,
-			boolean shouldIncludeJoins) {
+	private CollectionFetchableIndex buildIndexGraph() {
 		final CollectionPersister persister = collectionQuerySpace.getCollectionPersister();
 		if ( persister.hasIndex() ) {
 			final Type type = persister.getIndexType();
@@ -80,7 +103,7 @@ public abstract class AbstractCollectionReference implements CollectionReference
 							(EntityType) persister.getIndexType(),
 							collectionQuerySpace.getExpandingQuerySpaces().generateImplicitUid(),
 							collectionQuerySpace.canJoinsBeRequired(),
-							shouldIncludeJoins
+							allowIndexJoin
 					);
 					return new CollectionFetchableIndexEntityGraph( this, entityQuerySpace );
 				}
@@ -100,7 +123,7 @@ public abstract class AbstractCollectionReference implements CollectionReference
 						(CompositeType) persister.getIndexType(),
 						collectionQuerySpace.getExpandingQuerySpaces().generateImplicitUid(),
 						collectionQuerySpace.canJoinsBeRequired(),
-						shouldIncludeJoins
+						allowIndexJoin
 				);
 				return new CollectionFetchableIndexCompositeGraph( this, compositeQuerySpace );
 			}
@@ -109,9 +132,7 @@ public abstract class AbstractCollectionReference implements CollectionReference
 		return null;
 	}
 
-	private CollectionFetchableElement buildElementGraph(
-			ExpandingCollectionQuerySpace collectionQuerySpace,
-			boolean shouldIncludeJoins) {
+	private CollectionFetchableElement buildElementGraph() {
 		final CollectionPersister persister = collectionQuerySpace.getCollectionPersister();
 		final Type type = persister.getElementType();
 		if ( type.isAssociationType() ) {
@@ -126,7 +147,7 @@ public abstract class AbstractCollectionReference implements CollectionReference
 						(EntityType) persister.getElementType(),
 						collectionQuerySpace.getExpandingQuerySpaces().generateImplicitUid(),
 						collectionQuerySpace.canJoinsBeRequired(),
-						shouldIncludeJoins
+						allowElementJoin
 				);
 				return new CollectionFetchableElementEntityGraph( this, entityQuerySpace );
 			}
@@ -146,7 +167,7 @@ public abstract class AbstractCollectionReference implements CollectionReference
 					(CompositeType) persister.getElementType(),
 					collectionQuerySpace.getExpandingQuerySpaces().generateImplicitUid(),
 					collectionQuerySpace.canJoinsBeRequired(),
-					shouldIncludeJoins
+					allowElementJoin
 			);
 			return new CollectionFetchableElementCompositeGraph( this, compositeQuerySpace );
 		}
@@ -154,6 +175,16 @@ public abstract class AbstractCollectionReference implements CollectionReference
 		return null;
 	}
 
+	@Override
+	public boolean allowElementJoin() {
+		return allowElementJoin;
+	}
+
+	@Override
+	public boolean allowIndexJoin() {
+		return allowIndexJoin;
+	}
+
 	@Override
 	public String getQuerySpaceUid() {
 		return collectionQuerySpace.getUid();

hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractCollectionLoadQueryDetails.java

@@ -86,14 +86,13 @@ public abstract class AbstractCollectionLoadQueryDetails extends AbstractLoadQue
 				new CollectionReturnReader( rootReturn ),
 				new CollectionReferenceInitializerImpl( rootReturn, collectionReferenceAliases )
 		);
-		if ( rootReturn.getCollectionPersister().getElementType().isEntityType() ) {
+		if ( rootReturn.allowElementJoin() && rootReturn.getCollectionPersister().getElementType().isEntityType() ) {
 			final EntityReference elementEntityReference = rootReturn.getElementGraph().resolveEntityReference();
 			readerCollector.add(
 				new EntityReferenceInitializerImpl( elementEntityReference, collectionReferenceAliases.getEntityElementAliases() )
 			);
 		}
-		if ( rootReturn.getCollectionPersister().hasIndex() &&
-				rootReturn.getCollectionPersister().getIndexType().isEntityType() ) {
+		if ( rootReturn.allowIndexJoin() && rootReturn.getCollectionPersister().getIndexType().isEntityType() ) {
 			final EntityReference indexEntityReference = rootReturn.getIndexGraph().resolveEntityReference();
 			final EntityReferenceAliases indexEntityReferenceAliases = aliasResolutionContext.generateEntityReferenceAliases(
 					indexEntityReference.getQuerySpaceUid(),
@@ -134,8 +133,7 @@ public abstract class AbstractCollectionLoadQueryDetails extends AbstractLoadQue
 
 	@Override
 	protected  void applyRootReturnSelectFragments(SelectStatementBuilder selectStatementBuilder) {
-		if ( getQueryableCollection().hasIndex() &&
-				getQueryableCollection().getIndexType().isEntityType() ) {
+		if ( getRootCollectionReturn().allowIndexJoin() && getQueryableCollection().getIndexType().isEntityType() ) {
 			final EntityReference indexEntityReference = getRootCollectionReturn().getIndexGraph().resolveEntityReference();
 			final EntityReferenceAliases indexEntityReferenceAliases = getAliasResolutionContext().resolveEntityReferenceAliases(
 					indexEntityReference.getQuerySpaceUid()

hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionReference.java

@@ -77,4 +77,20 @@ public interface CollectionReference {
 	 * @return The PropertyPath
 	 */
 	public PropertyPath getPropertyPath();
+
+	/**
+	 * Should a collection element join be allowed? Returning <code>true</code>
+	 * indicates that an element join can safely be added.
+	 *
+	 * @return true, if a collection index join is allowed.
+	 */
+	public boolean allowElementJoin();
+
+	/**
+	 * Should a collection index join be allowed? Returning <code>true</code>
+	 * indicates that an index join can safely be added.
+	 *
+	 * @return true, if a collection index join is allowed.
+	 */
+	public boolean allowIndexJoin();
 }

hibernate-core/src/main/java/org/hibernate/persister/walking/internal/CompositionSingularSubAttributesHelper.java

@@ -109,7 +109,7 @@ public final class CompositionSingularSubAttributesHelper {
 				(OuterJoinLoadable) collectionPersister.getOwnerEntityPersister(),
 				(CompositeType) collectionPersister.getIndexType(),
 				collectionPersister.getTableName(),
-				collectionPersister.getIndexColumnNames()
+				collectionPersister.toColumns( "index" )
 		);
 	}
 

hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java

@@ -144,7 +144,7 @@ public class MetamodelGraphWalker {
 		if ( attributeDefinitions == null ) {
 			return;
 		}
-		for ( AttributeDefinition attributeDefinition : attributeSource.getAttributes() ) {
+		for ( AttributeDefinition attributeDefinition : attributeDefinitions ) {
 			visitAttributeDefinition( attributeDefinition );
 		}
 	}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/Currency.java

@@ -0,0 +1,37 @@
+//$Id$
+package org.hibernate.test.annotations.indexcoll;
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+
+@Entity
+public class Currency {
+	private Integer id;
+
+
+	@Id
+	@GeneratedValue
+	public Integer getId() {
+		return id;
+	}
+
+	public void setId(Integer id) {
+		this.id = id;
+	}
+
+
+	@Column
+	private String currency;
+
+
+	public String getCurrency() {
+		return currency;
+	}
+
+	public void setCurrency(String currency) {
+		this.currency = currency;
+	}
+	
+}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeOffice.java

@@ -0,0 +1,48 @@
+//$Id$
+package org.hibernate.test.annotations.indexcoll;
+import java.math.BigDecimal;
+import java.util.Map;
+
+import javax.persistence.ElementCollection;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+import org.hibernate.annotations.CascadeType;
+
+
+@Entity
+public class ExchangeOffice {
+	public ExchangeOffice() {
+		super();
+	}
+
+	@Id @GeneratedValue
+	private Integer id;
+	
+
+	public void setId(Integer id) {
+		this.id = id;
+	}
+
+	
+	public Integer getId() {
+		return id;
+	}
+
+	@javax.persistence.OneToMany(mappedBy = "parent")
+    @javax.persistence.MapKey(name="key")
+    private Map<ExchangeRateKey, ExchangeRate> exchangeRates = new java.util.HashMap<ExchangeRateKey, ExchangeRate>();
+	
+	public Map<ExchangeRateKey,ExchangeRate> getExchangeRates() {
+		return exchangeRates;
+	}
+
+	@ElementCollection
+	private Map<ExchangeRateKey, BigDecimal> exchangeRateFees = new java.util.HashMap<ExchangeRateKey, BigDecimal>();
+
+	public Map<ExchangeRateKey,BigDecimal> getExchangeRateFees() {
+		return exchangeRateFees;
+	}
+	
+}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRate.java

@@ -0,0 +1,63 @@
+//$Id$
+package org.hibernate.test.annotations.indexcoll;
+import javax.persistence.Column;
+import javax.persistence.Embedded;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+
+@Entity
+public class ExchangeRate {
+	public ExchangeRate() {
+		super();
+	}
+
+	@Id @GeneratedValue
+	private Integer id;
+	
+	@Column
+	private double rate;
+	
+
+	
+	public double getRate() {
+		return rate;
+	}
+
+	public void setRate(double rate) {
+		this.rate = rate;
+	}
+
+	public Integer getId() {
+		return id;
+	}
+
+	public void setId(Integer id) {
+		this.id = id;
+	}
+
+	@Embedded
+    private ExchangeRateKey key = new ExchangeRateKey();
+
+	public ExchangeRateKey getKey() {
+		return key;
+	}
+
+	public void setKey(ExchangeRateKey key) {
+		this.key = key;
+	}
+
+	@javax.persistence.ManyToOne(fetch = FetchType.LAZY )
+    private ExchangeOffice parent = null;
+
+
+	public ExchangeOffice getParent() {
+		return parent;
+	}
+
+	public void setParent(ExchangeOffice parent) {
+		this.parent = parent;
+	}
+}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRateKey.java

@@ -0,0 +1,56 @@
+//$Id$
+package org.hibernate.test.annotations.indexcoll;
+import javax.persistence.Column;
+import javax.persistence.Embeddable;
+import javax.persistence.FetchType;
+
+
+@Embeddable
+public class ExchangeRateKey
+{
+
+    public ExchangeRateKey() {
+        super();
+    }
+
+  
+    public ExchangeRateKey( long date, Currency currency1, Currency currency2) {
+        super();
+        this.date = date;
+        this.currency1 = currency1;
+        this.currency2 = currency2;
+    }
+
+    @Column(nullable = false)
+    protected long date;
+    
+    @javax.persistence.ManyToOne(fetch = FetchType.LAZY )
+    protected Currency currency1;
+    
+    @javax.persistence.ManyToOne(fetch = FetchType.LAZY )
+    protected Currency currency2;
+    
+
+    @Override
+    public boolean equals (Object obj) {
+        if (this == obj)  return true;
+
+        if (!(obj instanceof ExchangeRateKey)) return false;
+
+        ExchangeRateKey q = (ExchangeRateKey) obj;
+        return q.date == date && q.currency1 == this.currency1 && q.currency2 == this.currency2;
+
+    }
+
+
+
+    @Override
+    public int hashCode() {
+        int hashcode = 0;
+        hashcode += date;
+        hashcode += (currency1 != null ? currency1.hashCode() : 0);
+        hashcode += (currency2 != null ? currency2.hashCode() : 0);
+        return hashcode;
+    }
+
+}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/IndexedCollectionTest.java

@@ -33,9 +33,11 @@ import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Column;
 import org.hibernate.testing.RequiresDialect;
 import org.hibernate.testing.SkipForDialect;
+import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
 import org.junit.Test;
 
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
@@ -562,6 +564,95 @@ public class IndexedCollectionTest extends BaseNonConfigCoreFunctionalTestCase {
 		s.close();
 	}
 
+	@Test
+	@TestForIssue( jiraKey = "HHH-8879")
+	public void testMapKeyEmbeddableWithEntityKey() throws Exception {
+		Session s;
+		Transaction tx;
+		s = openSession();
+		tx = s.beginTransaction();
+		Currency currency1= new Currency();
+		Currency currency2= new Currency();
+		s.persist( currency1 );
+		s.persist( currency2 );
+		Integer id1 = currency1.getId();
+		Integer id2 = currency2.getId();
+		ExchangeRateKey cq = new ExchangeRateKey(20140101, currency1, currency2);
+
+		ExchangeRate m = new ExchangeRate();
+		m.setKey( cq );
+		s.persist( m );
+		ExchangeOffice wm = new ExchangeOffice();
+		s.persist( wm );
+
+		wm.getExchangeRates().put( cq, m );
+		m.setParent( wm );
+		Integer id = wm.getId();
+		s.flush();
+		tx.commit();
+		s.close();
+
+		s = openSession();
+		tx = s.beginTransaction();
+		wm = (ExchangeOffice) s.byId(ExchangeOffice.class).load(id);
+		assertNotNull(wm);
+		wm.getExchangeRates().size();
+		currency1 = (Currency) s.byId(Currency.class).load(id1);
+		assertNotNull(currency1);
+		currency2 = (Currency) s.byId(Currency.class).load(id2);
+		assertNotNull(currency2);
+		cq = new ExchangeRateKey(20140101, currency1, currency2);
+
+		m = wm.getExchangeRates().get( cq );
+		assertNotNull(m);
+		tx.commit();
+		s.close();
+
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-8994")
+	public void testEmbeddableWithEntityKey() throws Exception {
+		Session s;
+		Transaction tx;
+		s = openSession();
+		tx = s.beginTransaction();
+		Currency currency1= new Currency();
+		Currency currency2= new Currency();
+		s.persist( currency1 );
+		s.persist( currency2 );
+		Integer id1 = currency1.getId();
+		Integer id2 = currency2.getId();
+		ExchangeRateKey cq = new ExchangeRateKey(20140101, currency1, currency2);
+
+		ExchangeOffice wm = new ExchangeOffice();
+		s.persist( wm );
+
+		final BigDecimal fee = BigDecimal.valueOf( 12, 2 );
+
+		wm.getExchangeRateFees().put( cq, fee );
+		Integer id = wm.getId();
+		s.flush();
+		tx.commit();
+		s.close();
+
+		s = openSession();
+		tx = s.beginTransaction();
+		wm = (ExchangeOffice) s.byId(ExchangeOffice.class).load(id);
+		assertNotNull(wm);
+		wm.getExchangeRateFees().size();
+		currency1 = (Currency) s.byId(Currency.class).load(id1);
+		assertNotNull(currency1);
+		currency2 = (Currency) s.byId(Currency.class).load(id2);
+		assertNotNull(currency2);
+		cq = new ExchangeRateKey(20140101, currency1, currency2);
+
+		assertEquals( fee, wm.getExchangeRateFees().get( cq ) );
+
+		tx.commit();
+		s.close();
+	}
+
 	@Test
 	public void testEntityKeyElementTarget() throws Exception {
 		Session s = openSession();
@@ -670,7 +761,10 @@ public class IndexedCollectionTest extends BaseNonConfigCoreFunctionalTestCase {
 				AlphabeticalDirectory.class,
 				GasKey.class,
 				Trainee.class,
-				Training.class
+				Training.class,
+				Currency.class,
+				ExchangeOffice.class,
+				ExchangeRate.class,
 		};
 	}
 }