HHH-11005 - OneToMany generates incorrect SQL where MapKey is on supe…

…rclass and using InheritanceType.JOINED

hibernate-core/src/main/java/org/hibernate/cfg/annotations/MapBinder.java

@@ -12,6 +12,7 @@
 import java.util.Random;
 import javax.persistence.AttributeOverride;
 import javax.persistence.AttributeOverrides;
+import javax.persistence.InheritanceType;
 import javax.persistence.MapKeyClass;
 
 import org.hibernate.AnnotationException;
@@ -30,6 +31,7 @@
 import org.hibernate.cfg.CollectionSecondPass;
 import org.hibernate.cfg.Ejb3Column;
 import org.hibernate.cfg.Ejb3JoinColumn;
+import org.hibernate.cfg.InheritanceState;
 import org.hibernate.cfg.PropertyData;
 import org.hibernate.cfg.PropertyHolderBuilder;
 import org.hibernate.cfg.PropertyPreloadedData;
@@ -46,6 +48,7 @@
 import org.hibernate.mapping.OneToMany;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
+import org.hibernate.mapping.Selectable;
 import org.hibernate.mapping.SimpleValue;
 import org.hibernate.mapping.ToOne;
 import org.hibernate.mapping.Value;
@@ -121,8 +124,13 @@ private void bindKeyFromAssociationTable(
 				);
 			}
 			org.hibernate.mapping.Map map = (org.hibernate.mapping.Map) this.collection;
+			// HHH-11005 - if InheritanceType.JOINED then need to find class defining the column
+			InheritanceState inheritanceState = inheritanceStatePerClass.get( collType );
+			PersistentClass targetPropertyPersistentClass = InheritanceType.JOINED.equals( inheritanceState.getType() ) ?
+					mapProperty.getPersistentClass() :
+					associatedClass;
 			Value indexValue = createFormulatedValue(
-					mapProperty.getValue(), map, targetPropertyName, associatedClass, buildingContext
+					mapProperty.getValue(), map, targetPropertyName, associatedClass, targetPropertyPersistentClass, buildingContext
 			);
 			map.setIndex( indexValue );
 		}
@@ -305,6 +313,7 @@ protected Value createFormulatedValue(
 			Collection collection,
 			String targetPropertyName,
 			PersistentClass associatedClass,
+			PersistentClass targetPropertyPersistentClass,
 			MetadataBuildingContext buildingContext) {
 		Value element = collection.getElement();
 		String fromAndWhere = null;
@@ -322,28 +331,31 @@ else if ( element instanceof DependantValue ) {
 					throw new AnnotationException( "SecondaryTable JoinColumn cannot reference a non primary key" );
 				}
 			}
-			Iterator referencedEntityColumns;
+			Iterator<Selectable> referencedEntityColumns;
 			if ( referencedPropertyName == null ) {
 				referencedEntityColumns = associatedClass.getIdentifier().getColumnIterator();
 			}
 			else {
 				Property referencedProperty = associatedClass.getRecursiveProperty( referencedPropertyName );
 				referencedEntityColumns = referencedProperty.getColumnIterator();
 			}
-			String alias = "$alias$";
-			StringBuilder fromAndWhereSb = new StringBuilder( " from " )
-					.append( associatedClass.getTable().getName() )
-							//.append(" as ") //Oracle doesn't support it in subqueries
-					.append( " " )
-					.append( alias ).append( " where " );
-			Iterator collectionTableColumns = element.getColumnIterator();
-			while ( collectionTableColumns.hasNext() ) {
-				Column colColumn = (Column) collectionTableColumns.next();
-				Column refColumn = (Column) referencedEntityColumns.next();
-				fromAndWhereSb.append( alias ).append( '.' ).append( refColumn.getQuotedName() )
-						.append( '=' ).append( colColumn.getQuotedName() ).append( " and " );
+			fromAndWhere = getFromAndWhereFormula(
+					associatedClass.getTable().getName(),
+					element.getColumnIterator(),
+					referencedEntityColumns
+			);
+		}
+		else {
+			// HHH-11005 - only if we are OneToMany and location of map key property is at a different level, need to add a select
+			if ( !associatedClass.equals( targetPropertyPersistentClass ) ) {
+				fromAndWhere = getFromAndWhereFormula(
+						targetPropertyPersistentClass.getTable()
+								.getQualifiedTableName()
+								.toString(),
+						element.getColumnIterator(),
+						associatedClass.getIdentifier().getColumnIterator()
+				);
 			}
-			fromAndWhere = fromAndWhereSb.substring( 0, fromAndWhereSb.length() - 5 );
 		}
 
 		if ( value instanceof Component ) {
@@ -368,7 +380,7 @@ else if ( element instanceof DependantValue ) {
 				newProperty.setSelectable( current.isSelectable() );
 				newProperty.setValue(
 						createFormulatedValue(
-								current.getValue(), collection, targetPropertyName, associatedClass, buildingContext
+								current.getValue(), collection, targetPropertyName, associatedClass, associatedClass, buildingContext
 						)
 				);
 				indexComponent.addProperty( newProperty );
@@ -425,4 +437,27 @@ else if ( current instanceof Formula ) {
 			throw new AssertionFailure( "Unknown type encounters for map key: " + value.getClass() );
 		}
 	}
+
+	private String getFromAndWhereFormula(
+			String tableName,
+			Iterator<Selectable> collectionTableColumns,
+			Iterator<Selectable> referencedEntityColumns) {
+		String alias = "$alias$";
+		StringBuilder fromAndWhereSb = new StringBuilder( " from " )
+				.append( tableName )
+				//.append(" as ") //Oracle doesn't support it in subqueries
+				.append( " " )
+				.append( alias ).append( " where " );
+		while ( collectionTableColumns.hasNext() ) {
+			Column colColumn = (Column) collectionTableColumns.next();
+			Column refColumn = (Column) referencedEntityColumns.next();
+			fromAndWhereSb.append( alias )
+					.append( '.' )
+					.append( refColumn.getQuotedName() )
+					.append( '=' )
+					.append( colColumn.getQuotedName() )
+					.append( " and " );
+		}
+		return fromAndWhereSb.substring( 0, fromAndWhereSb.length() - 5 );
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/onetomany/inheritance/joined/Book.java

@@ -0,0 +1,36 @@
+package org.hibernate.test.onetomany.inheritance.joined;
+
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
+@Entity
+@Table(name="BOOKTABJO")
+public class Book extends Product {
+
+	private String isbn;
+
+	@ManyToOne
+	private Library library;
+
+	public Book() {
+		super();
+	}
+
+	public Book(String inventoryCode, String isbn) {
+		super(inventoryCode);
+		this.isbn = isbn;
+	}
+
+	public String getIsbn() {
+		return isbn;
+	}
+
+	public Library getLibrary() {
+		return library;
+	}
+
+	public void setLibrary(Library library) {
+		this.library = library;
+	}
+}

hibernate-core/src/test/java/org/hibernate/test/onetomany/inheritance/joined/Library.java

@@ -0,0 +1,52 @@
+package org.hibernate.test.onetomany.inheritance.joined;
+
+import java.util.HashMap;
+import java.util.Map;
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.MapKey;
+import javax.persistence.OneToMany;
+import javax.persistence.Table;
+
+@Entity
+@Table(name="LIBRARYJO")
+public class Library {
+
+	@Id
+	@GeneratedValue
+	private int entid;
+
+	@OneToMany(mappedBy="library", cascade = CascadeType.ALL)
+	@MapKey(name="inventoryCode")
+	private Map<String,Book> booksOnInventory = new HashMap<>();
+
+	@OneToMany(mappedBy="library", cascade = CascadeType.ALL)
+	@MapKey(name="isbn")
+	private Map<String,Book> booksOnIsbn = new HashMap<>();
+
+	public int getEntid() {
+		return entid;
+	}
+
+	public Map<String,Book> getBooksOnInventory() {
+		return booksOnInventory;
+	}
+
+	public Map<String, Book> getBooksOnIsbn() {
+		return booksOnIsbn;
+	}
+
+	public void addBook(Book book) {
+		book.setLibrary( this );
+		booksOnInventory.put( book.getInventoryCode(), book );
+		booksOnIsbn.put( book.getIsbn(), book );
+	}
+
+	public void removeBook(Book book) {
+		book.setLibrary( null );
+		booksOnInventory.remove( book.getInventoryCode() );
+		booksOnIsbn.remove( book.getIsbn() );
+	}
+}

hibernate-core/src/test/java/org/hibernate/test/onetomany/inheritance/joined/MappedSuperclassMapTest.java

@@ -0,0 +1,142 @@
+package org.hibernate.test.onetomany.inheritance.joined;
+
+import java.util.List;
+import java.util.Map.Entry;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.jboss.logging.Logger;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+@TestForIssue(jiraKey = "HHH-11005")
+public class MappedSuperclassMapTest extends BaseNonConfigCoreFunctionalTestCase {
+
+	private static final Logger log = Logger.getLogger( MappedSuperclassMapTest.class );
+
+	private static final String SKU001 = "SKU001";
+	private static final String SKU002 = "SKU002";
+	private static final String WAR_AND_PEACE = "0140447938";
+	private static final String ANNA_KARENINA = "0140449175";
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[]{
+			Book.class,
+			Library.class,
+			Product.class
+		};
+	}
+
+	@Before
+	public void init() {
+		doInHibernate( this::sessionFactory, sess -> {
+			Book book1 = new Book( SKU001, WAR_AND_PEACE);
+			Book book2 = new Book( SKU002, ANNA_KARENINA);
+			sess.persist( book1 );
+			sess.flush();
+			sess.persist( book2 );
+			sess.flush();
+			Library library = new Library();
+			library.addBook( book1 );
+			library.addBook( book2 );
+			sess.persist(library);
+		} );
+	}
+
+	@Test
+	public void lookupEntities() {
+		doInHibernate( this::sessionFactory, sess -> {
+			List<Library> libraries = sess.createQuery( "FROM Library").list();
+			assertEquals(1, libraries.size());
+			Library library = libraries.get( 0);
+			assertNotNull(library);
+
+			assertEquals(2, library.getBooksOnInventory().size());
+
+			Book book = library.getBooksOnInventory().get( SKU001);
+			assertNotNull(book);
+			Library Library = library;
+			Library.getBooksOnIsbn().get( WAR_AND_PEACE );
+			assertEquals(WAR_AND_PEACE, book.getIsbn());
+
+			book = library.getBooksOnInventory().get(SKU002);
+			assertNotNull(book);
+			assertEquals(ANNA_KARENINA, book.getIsbn());
+		} );
+	}
+
+	@Test
+	public void lookupEntities_entrySet() {
+		doInHibernate( this::sessionFactory, sess -> {
+			List<Library> libraries = sess.createQuery( "FROM Library").list();
+			assertEquals(1, libraries.size());
+			Library library = libraries.get( 0);
+			assertNotNull(library);
+
+			assertEquals(2, library.getBooksOnInventory().size());
+
+			for (Entry<String,Book> entry : library.getBooksOnInventory().entrySet()) {
+				log.info("Found SKU " + entry.getKey() + " with ISBN " + entry.getValue().getIsbn());
+			}
+		} );
+	}
+
+	@Test
+	public void breakReferences() {
+		doInHibernate( this::sessionFactory, sess -> {
+			List<Book> books = sess.createQuery( "FROM Book").list();
+			assertEquals(2, books.size());
+
+			for (Book book : books) {
+				assertNotNull(book.getLibrary());
+				log.info("Found SKU " + book.getInventoryCode() + " with library " + book.getLibrary().getEntid());
+			}
+
+			for (Book book : books) {
+				book.getLibrary().removeBook( book );
+			}
+		} );
+		doInHibernate( this::sessionFactory, sess -> {
+			List<Book> books = sess.createQuery( "FROM Book").list();
+			assertEquals(2, books.size());
+
+			for (Book book : books) {
+				if (book.getLibrary() == null ) {
+					log.info("Found SKU " + book.getInventoryCode() + " with no library");
+				}
+			}
+
+			List<Library> libraries = sess.createQuery( "FROM Library").list();
+			assertEquals(1, libraries.size());
+			Library library = libraries.get( 0);
+			assertNotNull(library);
+
+			assertEquals(0, library.getBooksOnInventory().size());
+			log.info("Found Library " + library.getEntid() + " with no books");
+		} );
+	}
+
+	@Override
+	protected boolean isCleanupTestDataRequired() {
+		return true;
+	}
+
+	@Override
+	protected void cleanupTestData() throws Exception {
+		doInHibernate( this::sessionFactory, sess -> {
+			sess.createQuery( "delete from Book" ).executeUpdate();
+			sess.createQuery( "delete from Library" ).executeUpdate();
+		} );
+	}
+
+	@Override
+	protected boolean rebuildSessionFactoryOnError() {
+		return false;
+	}
+}