HHH-8839 : Eager Map with entity key causes IllegalStateException: Collection element (many-to-many) table alias cannot be empty

(cherry picked from commit 35edd56907)

hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/spaces/CollectionQuerySpaceImpl.java

@@ -26,6 +26,7 @@ package org.hibernate.loader.plan.build.internal.spaces;
 import org.hibernate.loader.plan.build.spi.ExpandingCollectionQuerySpace;
 import org.hibernate.loader.plan.build.spi.ExpandingQuerySpaces;
 import org.hibernate.loader.plan.spi.Join;
+import org.hibernate.loader.plan.spi.JoinDefinedByMetadata;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.collection.CollectionPropertyNames;
 import org.hibernate.persister.collection.QueryableCollection;
@@ -36,6 +37,8 @@ import org.hibernate.persister.entity.PropertyMapping;
  */
 public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements ExpandingCollectionQuerySpace {
 	private final CollectionPersister persister;
+	private JoinDefinedByMetadata elementJoin;
+	private JoinDefinedByMetadata indexJoin;
 
 	public CollectionQuerySpaceImpl(
 			CollectionPersister persister,
@@ -78,6 +81,35 @@ public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements Expa
 
 	@Override
 	public void addJoin(Join join) {
+		if ( JoinDefinedByMetadata.class.isInstance( join ) ) {
+			final JoinDefinedByMetadata joinDefinedByMetadata = (JoinDefinedByMetadata) join;
+			if ( joinDefinedByMetadata.getJoinedPropertyName().equals( CollectionPropertyNames.COLLECTION_ELEMENTS ) ) {
+				if ( elementJoin == null ) {
+					elementJoin = joinDefinedByMetadata;
+				}
+				else {
+					throw new IllegalStateException( "Attempt to add an element join, but an element join already exists." );
+				}
+			}
+			else if ( joinDefinedByMetadata.getJoinedPropertyName().equals( CollectionPropertyNames.COLLECTION_INDICES ) ) {
+				if ( indexJoin == null ) {
+					indexJoin = joinDefinedByMetadata;
+				}
+				else {
+					throw new IllegalStateException( "Attempt to add an index join, but an index join already exists." );
+				}
+			}
+			else {
+				throw new IllegalArgumentException(
+						String.format(
+								"Collection propertyName must be either %s or %s; instead the joined property name was %s.",
+								CollectionPropertyNames.COLLECTION_ELEMENTS,
+								CollectionPropertyNames.COLLECTION_INDICES,
+								joinDefinedByMetadata.getJoinedPropertyName()
+						)
+				);
+			}
+		}
 		internalGetJoins().add( join );
 	}
 
@@ -86,4 +118,7 @@ public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements Expa
 		return super.getExpandingQuerySpaces();
 	}
 
+	public void addJoin(JoinDefinedByMetadata join) {
+		addJoin( (Join) join );
+	}
 }

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

@@ -24,6 +24,9 @@
 package org.hibernate.loader.plan.build.spi;
 
 import org.hibernate.loader.plan.spi.CollectionQuerySpace;
+import org.hibernate.loader.plan.spi.Join;
+import org.hibernate.loader.plan.spi.JoinDefinedByMetadata;
+import org.hibernate.persister.collection.CollectionPropertyNames;
 
 /**
  * Describes a collection query space that allows adding joins with other
@@ -34,4 +37,19 @@ import org.hibernate.loader.plan.spi.CollectionQuerySpace;
  * @author Gail Badner
  */
 public interface ExpandingCollectionQuerySpace extends CollectionQuerySpace, ExpandingQuerySpace {
+
+	/**
+	 * Adds a join with another query space for either a collection element or index. If {@code join}
+	 * is an instance of {@link JoinDefinedByMetadata}, then the only valid values returned by
+	 * {@link JoinDefinedByMetadata#getJoinedPropertyName} are {@link CollectionPropertyNames#COLLECTION_ELEMENTS}
+	 * and {@link CollectionPropertyNames#COLLECTION_INDICES}, for the collection element or index, respectively.
+	 *
+	 * @param join The element or index join to add.
+	 *
+	 * @throws java.lang.IllegalArgumentException if {@code join} is an instance of {@link JoinDefinedByMetadata}
+	 * and {@code join.getJoinedPropertyName() is neither {@link CollectionPropertyNames#COLLECTION_ELEMENTS}
+	 * nor {@link CollectionPropertyNames#COLLECTION_INDICES}}.
+	 * @throws java.lang.IllegalStateException if there is already an existing join with the same joined property name.
+	 */
+	public void addJoin(Join join);
 }

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

@@ -23,6 +23,7 @@
  */
 package org.hibernate.loader.plan.exec.internal;
 
+import org.hibernate.AssertionFailure;
 import org.hibernate.engine.FetchStyle;
 import org.hibernate.engine.FetchTiming;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
@@ -284,28 +285,43 @@ public class LoadQueryJoinAndFetchProcessor {
 		// For many-to-many, the follow-on join will join to the associated entity element table. For one-to-many,
 		// the collection table is the associated entity table, so the follow-on join will not be rendered..
 
+		// currently we do not explicitly track the joins under the CollectionQuerySpace to know which is
+		// the element join and which is the index join (maybe we should?).
+
+		JoinDefinedByMetadata collectionElementJoin = null;
+		JoinDefinedByMetadata collectionIndexJoin = null;
+		for ( Join collectionJoin : rightHandSide.getJoins() ) {
+			if ( JoinDefinedByMetadata.class.isInstance( collectionJoin ) ) {
+				final JoinDefinedByMetadata collectionJoinDefinedByMetadata = (JoinDefinedByMetadata) collectionJoin;
+				if ( CollectionPropertyNames.COLLECTION_ELEMENTS.equals( collectionJoinDefinedByMetadata.getJoinedPropertyName() ) ) {
+					if ( collectionElementJoin != null ) {
+						throw new AssertionFailure(
+								String.format(
+										"More than one element join defined for: %s",
+										rightHandSide.getCollectionPersister().getRole()
+								)
+						);
+					}
+					collectionElementJoin = collectionJoinDefinedByMetadata;
+				}
+				if ( CollectionPropertyNames.COLLECTION_INDICES.equals( collectionJoinDefinedByMetadata.getJoinedPropertyName() ) ) {
+					if ( collectionIndexJoin != null ) {
+						throw new AssertionFailure(
+								String.format(
+										"More than one index join defined for: %s",
+										rightHandSide.getCollectionPersister().getRole()
+								)
+						);
+					}
+					collectionIndexJoin = collectionJoinDefinedByMetadata;
+				}
+			}
+		}
+
 		if ( rightHandSide.getCollectionPersister().isOneToMany()
 				|| rightHandSide.getCollectionPersister().isManyToMany() ) {
 			// relatedly, for collections with entity elements (one-to-many, many-to-many) we need to register the
 			// sql aliases to use for the entity.
-			//
-			// currently we do not explicitly track the joins under the CollectionQuerySpace to know which is
-			// the element join and which is the index join (maybe we should?).  Another option here is to have the
-			// "collection join" act as the entity element join in this case (much like I do with entity identifiers).
-			// The difficulty there is that collections can theoretically could be multiple joins in that case (one
-			// for element, one for index).  However, that's a bit of future-planning as today Hibernate does not
-			// properly deal with the index anyway in terms of allowing dynamic fetching across a collection index...
-			//
-			// long story short, for now we'll use an assumption that the last join in the CollectionQuerySpace is the
-			// element join (that's how the joins are built as of now..)
-			//
-			// todo : remove this assumption ^^; maybe we make CollectionQuerySpace "special" and rather than have it
-			// hold a list of joins, we have it expose the 2 (index, element) separately.
-
-			Join collectionElementJoin = null;
-			for ( Join collectionJoin : rightHandSide.getJoins() ) {
-				collectionElementJoin = collectionJoin;
-			}
 			if ( collectionElementJoin == null ) {
 				throw new IllegalStateException(
 						String.format(
@@ -329,6 +345,24 @@ public class LoadQueryJoinAndFetchProcessor {
 			);
 		}
 
+		if ( rightHandSide.getCollectionPersister().hasIndex() &&
+				rightHandSide.getCollectionPersister().getIndexType().isEntityType() ) {
+			// for collections with entity index we need to register the
+			// sql aliases to use for the entity.
+			if ( collectionIndexJoin == null ) {
+				throw new IllegalStateException(
+						String.format(
+								"Could not locate collection index join within collection join [%s : %s]",
+								rightHandSide.getUid(),
+								rightHandSide.getCollectionPersister()
+						)
+				);
+			}
+			aliasResolutionContext.generateEntityReferenceAliases(
+					collectionIndexJoin.getRightHandSide().getUid(),
+					rightHandSide.getCollectionPersister().getIndexDefinition().toEntityDefinition().getEntityPersister()
+			);
+		}
 		addJoins(
 				join,
 				joinFragment,

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/Atmosphere.java

@@ -0,0 +1,84 @@
+//$Id$
+package org.hibernate.test.annotations.indexcoll.eager;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import javax.persistence.CascadeType;
+import javax.persistence.Column;
+import javax.persistence.ElementCollection;
+import javax.persistence.Entity;
+import javax.persistence.EnumType;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.JoinTable;
+import javax.persistence.ManyToMany;
+import javax.persistence.MapKeyColumn;
+import javax.persistence.MapKeyEnumerated;
+import javax.persistence.MapKeyJoinColumn;
+import javax.persistence.MapKeyJoinColumns;
+import javax.persistence.MapKeyTemporal;
+import javax.persistence.TemporalType;
+
+import org.hibernate.test.annotations.indexcoll.Gas;
+import org.hibernate.test.annotations.indexcoll.GasKey;
+
+/**
+ * @author Emmanuel Bernard
+ */
+@Entity
+public class Atmosphere {
+
+	public static enum Level {
+		LOW,
+		HIGH
+	}
+
+	@Id
+	@GeneratedValue
+	public Integer id;
+
+	@ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
+	@MapKeyColumn(name="gas_name")
+	public Map<String, Gas> gases = new HashMap<String, Gas>();
+
+	@MapKeyTemporal(TemporalType.DATE)
+	@ElementCollection(fetch = FetchType.EAGER)
+	@MapKeyColumn(nullable=false)
+	public Map<Date, String> colorPerDate = new HashMap<Date,String>();
+
+	@ElementCollection(fetch = FetchType.EAGER)
+	@MapKeyEnumerated(EnumType.STRING)
+	@MapKeyColumn(nullable=false)
+	public Map<Level, String> colorPerLevel = new HashMap<Level,String>();
+
+	@ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
+	@MapKeyJoinColumn(name="gas_id" )
+	@JoinTable(name = "Gas_per_key")
+	public Map<GasKey, Gas> gasesPerKey = new HashMap<GasKey, Gas>();
+
+	@ElementCollection(fetch = FetchType.EAGER)
+	@Column(name="composition_rate")
+	@MapKeyJoinColumns( { @MapKeyJoinColumn(name="gas_id" ) } ) //use @MapKeyJoinColumns explicitly for tests
+	@JoinTable(name = "Composition", joinColumns = @JoinColumn(name = "atmosphere_id"))
+	public Map<Gas, Double> composition = new HashMap<Gas, Double>();
+
+	//use default JPA 2 column name for map key
+	@ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
+	@MapKeyColumn
+	@JoinTable(name="Atm_Gas_Def")
+	public Map<String, Gas> gasesDef = new HashMap<String, Gas>();
+
+	//use default HAN legacy column name for map key
+	@ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
+	@MapKeyColumn
+	@JoinTable(name="Atm_Gas_DefLeg")
+	public Map<String, Gas> gasesDefLeg = new HashMap<String, Gas>();
+
+	@ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
+	@MapKeyJoinColumn
+	@JoinTable(name = "Gas_p_key_def")
+	public Map<GasKey, Gas> gasesPerKeyDef = new HashMap<GasKey, Gas>();
+
+}

hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/EagerIndexedCollectionTest.java

@@ -0,0 +1,170 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2011, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.indexcoll.eager;
+
+import java.util.Date;
+import java.util.Iterator;
+
+import org.junit.Test;
+
+import org.hibernate.Hibernate;
+import org.hibernate.Session;
+import org.hibernate.Transaction;
+import org.hibernate.mapping.Collection;
+import org.hibernate.mapping.Column;
+import org.hibernate.test.annotations.indexcoll.Gas;
+import org.hibernate.test.annotations.indexcoll.GasKey;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test index collections
+ *
+ * @author Emmanuel Bernard
+ */
+public class EagerIndexedCollectionTest extends BaseCoreFunctionalTestCase {
+	@Test
+	public void testJPA2DefaultMapColumns() throws Exception {
+		isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesDef", "_KEY" );
+		isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesPerKeyDef", "_KEY" );
+		isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesDefLeg", "_KEY" );
+	}
+
+	private void isDefaultKeyColumnPresent(String collectionOwner, String propertyName, String suffix) {
+		assertTrue( "Could not find " + propertyName + suffix,
+				isDefaultColumnPresent(collectionOwner, propertyName, suffix) );
+	}
+
+	private boolean isDefaultColumnPresent(String collectionOwner, String propertyName, String suffix) {
+		final Collection collection = configuration().getCollectionMapping( collectionOwner + "." + propertyName );
+		final Iterator columnIterator = collection.getCollectionTable().getColumnIterator();
+		boolean hasDefault = false;
+		while ( columnIterator.hasNext() ) {
+			Column column = (Column) columnIterator.next();
+			if ( (propertyName + suffix).equals( column.getName() ) ) hasDefault = true;
+		}
+		return hasDefault;
+	}
+
+	@Test
+	public void testRealMap() throws Exception {
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+		Atmosphere atm = new Atmosphere();
+		Atmosphere atm2 = new Atmosphere();
+		GasKey key = new GasKey();
+		key.setName( "O2" );
+		Gas o2 = new Gas();
+		o2.name = "oxygen";
+		atm.gases.put( "100%", o2 );
+		atm.gasesPerKey.put( key, o2 );
+		atm2.gases.put( "100%", o2 );
+		atm2.gasesPerKey.put( key, o2 );
+		s.persist( key );
+		s.persist( atm );
+		s.persist( atm2 );
+
+		s.flush();
+		s.clear();
+
+		atm = (Atmosphere) s.get( Atmosphere.class, atm.id );
+		key = (GasKey) s.get( GasKey.class, key.getName() );
+		assertEquals( 1, atm.gases.size() );
+		assertEquals( o2.name, atm.gases.get( "100%" ).name );
+		assertEquals( o2.name, atm.gasesPerKey.get( key ).name );
+		tx.rollback();
+		s.close();
+	}
+
+	@Test
+	public void testTemporalKeyMap() throws Exception {
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+		Atmosphere atm = new Atmosphere();
+		atm.colorPerDate.put( new Date(1234567000), "red" );
+		s.persist( atm );
+
+		s.flush();
+		s.clear();
+
+		atm = (Atmosphere) s.get( Atmosphere.class, atm.id );
+		assertEquals( 1, atm.colorPerDate.size() );
+		final Date date = atm.colorPerDate.keySet().iterator().next();
+		final long diff = new Date( 1234567000 ).getTime() - date.getTime();
+		assertTrue( "24h diff max", diff > 0 && diff < 24*60*60*1000 );
+		tx.rollback();
+		s.close();
+	}
+
+	@Test
+	public void testEnumKeyType() throws Exception {
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+		Atmosphere atm = new Atmosphere();
+		atm.colorPerLevel.put( Atmosphere.Level.HIGH, "red" );
+		s.persist( atm );
+
+		s.flush();
+		s.clear();
+
+		atm = (Atmosphere) s.get( Atmosphere.class, atm.id );
+		assertEquals( 1, atm.colorPerLevel.size() );
+		assertEquals( "red", atm.colorPerLevel.get( Atmosphere.Level.HIGH) );
+		tx.rollback();
+		s.close();
+	}
+
+	@Test
+	public void testEntityKeyElementTarget() throws Exception {
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+		Atmosphere atm = new Atmosphere();
+		Gas o2 = new Gas();
+		o2.name = "oxygen";
+		atm.composition.put( o2, 94.3 );
+		s.persist( o2 );
+		s.persist( atm );
+
+		s.flush();
+		s.clear();
+
+		atm = (Atmosphere) s.get( Atmosphere.class, atm.id );
+		assertTrue( Hibernate.isInitialized( atm.composition ) );
+		assertEquals( 1, atm.composition.size() );
+		assertEquals( o2.name, atm.composition.keySet().iterator().next().name );
+		tx.rollback();
+		s.close();
+	}
+
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class[]{
+				Atmosphere.class,
+				Gas.class,
+				GasKey.class
+		};
+	}
+}