diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/returns/AbstractCollectionReference.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/returns/AbstractCollectionReference.java index bffdcc4417..f668d464ea 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/returns/AbstractCollectionReference.java +++ b/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(); diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractCollectionLoadQueryDetails.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractCollectionLoadQueryDetails.java index b36988ce7e..3410aca74d 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractCollectionLoadQueryDetails.java +++ b/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() diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionReference.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionReference.java index e98ab19f61..92a0227f5c 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionReference.java +++ b/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 true + * 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 true + * indicates that an index join can safely be added. + * + * @return true, if a collection index join is allowed. + */ + public boolean allowIndexJoin(); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/walking/internal/CompositionSingularSubAttributesHelper.java b/hibernate-core/src/main/java/org/hibernate/persister/walking/internal/CompositionSingularSubAttributesHelper.java index e3ad7ca12d..5f99e554d6 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/walking/internal/CompositionSingularSubAttributesHelper.java +++ b/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" ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java index 4dbf1e4b69..c46a26178a 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java +++ b/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 ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/Currency.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/Currency.java new file mode 100644 index 0000000000..10a2a79a16 --- /dev/null +++ b/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; + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeOffice.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeOffice.java new file mode 100644 index 0000000000..fe9d6d1191 --- /dev/null +++ b/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 exchangeRates = new java.util.HashMap(); + + public Map getExchangeRates() { + return exchangeRates; + } + + @ElementCollection + private Map exchangeRateFees = new java.util.HashMap(); + + public Map getExchangeRateFees() { + return exchangeRateFees; + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRate.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRate.java new file mode 100644 index 0000000000..a2c9bfa533 --- /dev/null +++ b/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; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRateKey.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/ExchangeRateKey.java new file mode 100644 index 0000000000..d316f02bf8 --- /dev/null +++ b/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; + } + +} \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/IndexedCollectionTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/IndexedCollectionTest.java index 626e9a4e85..a797a64073 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/IndexedCollectionTest.java +++ b/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, }; } }