HHH-9222 - Bulk delete and element collections

Conflicts:
	hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java

@@ -79,34 +79,49 @@ public class DeleteExecutor extends BasicExecutor {
 				parameterSpecifications = new ArrayList<ParameterSpecification>();
 				idSubselectWhere = "";
 			}
-			
+
-			// If many-to-many, delete the FK row in the collection table.
+			// find plural attributes defined for the entity being deleted...
 			for ( Type type : persister.getPropertyTypes() ) {
-				if ( type.isCollectionType() ) {
+				if ( ! type.isCollectionType() ) {
-					final CollectionType cType = (CollectionType) type;
+					continue;
-					final AbstractCollectionPersister cPersister = (AbstractCollectionPersister) factory
-							.getCollectionPersister( cType.getRole() );
-					if ( cPersister.isManyToMany() ) {
-						if (persister.getIdentifierColumnNames().length > 1
-								&& !dialect.supportsTuplesInSubqueries()) {
-							LOG.warn( "This dialect is unable to cascade the delete into the many-to-many join table" +
-									" when the entity has multiple primary keys.  Either properly setup cascading on" +
-									" the constraints or manually clear the associations prior to deleting the entities." );
-						}
-						else {
-							final String idSubselect = "(select "
-									+ StringHelper.join( ", ", persister.getIdentifierColumnNames() ) + " from "
-									+ persister.getTableName() + idSubselectWhere + ")";
-							final String where = "(" + StringHelper.join( ", ", cPersister.getKeyColumnNames() )
-									+ ") in " + idSubselect;
-							final Delete delete = new Delete().setTableName( cPersister.getTableName() ).setWhere( where );
-							if ( factory.getSettings().isCommentsEnabled() ) {
-								delete.setComment( "delete FKs in join table" );
-							}
-							deletes.add( delete.toStatementString() );
-						}
-					}
 				}
+
+				// if the plural attribute maps to a "collection table" we need
+				// to remove the rows from that table corresponding to any
+				// owners we are about to delete.  "collection table" is
+				// (unfortunately) indicated in a number of ways, but here we
+				// are mainly concerned with:
+				//		1) many-to-many mappings
+				//		2) basic collection mappings
+				final CollectionType cType = (CollectionType) type;
+				final AbstractCollectionPersister cPersister = (AbstractCollectionPersister) factory
+							.getCollectionPersister( cType.getRole() );
+				final boolean hasCollectionTable = cPersister.isManyToMany()
+						|| !cPersister.getElementType().isAssociationType();
+				if ( !hasCollectionTable ) {
+					continue;
+				}
+
+				if ( persister.getIdentifierColumnNames().length > 1
+						&& !dialect.supportsTuplesInSubqueries() ) {
+					LOG.warn(
+							"This dialect is unable to cascade the delete into the many-to-many join table" +
+									" when the entity has multiple primary keys.  Either properly setup cascading on" +
+									" the constraints or manually clear the associations prior to deleting the entities."
+					);
+					continue;
+				}
+
+				final String idSubselect = "(select "
+						+ StringHelper.join( ", ", persister.getIdentifierColumnNames() ) + " from "
+						+ persister.getTableName() + idSubselectWhere + ")";
+				final String where = "(" + StringHelper.join( ", ", cPersister.getKeyColumnNames() )
+						+ ") in " + idSubselect;
+				final Delete delete = new Delete().setTableName( cPersister.getTableName() ).setWhere( where );
+				if ( factory.getSettings().isCommentsEnabled() ) {
+					delete.setComment( "bulk delete - collection table clean up (" + cPersister.getRole() + ")" );
+				}
+				deletes.add( delete.toStatementString() );
 			}
 		}
 		catch (RecognitionException e) {

hibernate-core/src/main/java/org/hibernate/hql/spi/TableBasedDeleteHandlerImpl.java

@@ -82,17 +82,39 @@ public class TableBasedDeleteHandlerImpl
 		final String idSubselect = generateIdSubselect( targetedPersister );
 		deletes = new ArrayList<String>();
 		
-		// If many-to-many, delete the FK row in the collection table.
+		// find plural attributes defined for the entity being deleted...
-		// This partially overlaps with DeleteExecutor, but it instead uses the temp table in the idSubselect.
+		//
+		// NOTE : this partially overlaps with DeleteExecutor, but it instead
+		// 		uses the temp table in the idSubselect.
 		for ( Type type : targetedPersister.getPropertyTypes() ) {
-			if ( type.isCollectionType() ) {
+			if ( ! type.isCollectionType() ) {
-				CollectionType cType = (CollectionType) type;
+				continue;
-				AbstractCollectionPersister cPersister = (AbstractCollectionPersister)factory.getCollectionPersister( cType.getRole() );
-				if ( cPersister.isManyToMany() ) {
-					deletes.add( generateDelete( cPersister.getTableName(),
-							cPersister.getKeyColumnNames(), idSubselect, "bulk delete - m2m join table cleanup"));
-				}
 			}
+
+			final CollectionType cType = (CollectionType) type;
+			final AbstractCollectionPersister cPersister = (AbstractCollectionPersister)factory.getCollectionPersister( cType.getRole() );
+
+			// if the plural attribute maps to a "collection table" we need
+			// to remove the rows from that table corresponding to any
+			// owners we are about to delete.  "collection table" is
+			// (unfortunately) indicated in a number of ways, but here we
+			// are mainly concerned with:
+			//		1) many-to-many mappings
+			//		2) basic collection mappings
+			final boolean hasCollectionTable = cPersister.isManyToMany()
+					|| !cPersister.getElementType().isAssociationType();
+			if ( !hasCollectionTable ) {
+				continue;
+			}
+
+			deletes.add(
+					generateDelete(
+							cPersister.getTableName(),
+							cPersister.getKeyColumnNames(),
+							idSubselect,
+							"bulk delete - collection table clean up (" + cPersister.getRole() + ")"
+					)
+			);
 		}
 
 		String[] tableNames = targetedPersister.getConstraintOrderedTableNameClosure();
@@ -102,7 +124,7 @@ public class TableBasedDeleteHandlerImpl
 			//      the difficulty is the ordering of the tables here vs the cascade attributes on the persisters ->
 			//          the table info gotten here should really be self-contained (i.e., a class representation
 			//          defining all the needed attributes), then we could then get an array of those
-			deletes.add( generateDelete( tableNames[i], columnNames[i], idSubselect, "bulk delete"));
+			deletes.add( generateDelete( tableNames[i], columnNames[i], idSubselect, "bulk delete" ) );
 		}
 	}
 	

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

@@ -23,16 +23,15 @@
  */
 package org.hibernate.test.hql;
 
-import static org.junit.Assert.assertEquals;
+import java.sql.Connection;
-import static org.junit.Assert.assertNotNull;
+import java.sql.ResultSet;
-import static org.junit.Assert.assertTrue;
+import java.sql.SQLException;
-import static org.junit.Assert.fail;
+import java.sql.Statement;
-
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
-
+import java.util.TreeSet;
-import junit.framework.AssertionFailedError;
 
 import org.hibernate.QueryException;
 import org.hibernate.Session;
@@ -42,13 +41,21 @@ import org.hibernate.dialect.MySQLDialect;
 import org.hibernate.exception.ConstraintViolationException;
 import org.hibernate.id.BulkInsertionCapableIdentifierGenerator;
 import org.hibernate.id.IdentifierGenerator;
+import org.hibernate.jdbc.Work;
 import org.hibernate.persister.entity.EntityPersister;
+
 import org.hibernate.testing.DialectChecks;
 import org.hibernate.testing.RequiresDialectFeature;
 import org.hibernate.testing.SkipLog;
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
 import org.junit.Test;
+import junit.framework.AssertionFailedError;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Tests execution of bulk UPDATE/DELETE statements through the new AST parser.
@@ -1263,6 +1270,144 @@ public class BulkManipulationTest extends BaseCoreFunctionalTestCase {
 		}
 	}
 
+	@Test
+	@TestForIssue( jiraKey = "HHH-9222" )
+	public void testBulkDeleteOfEntityWithElementCollection() {
+		// set up test data
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			Farm farm = new Farm();
+			farm.setName( "Old McDonald Farm 'o the Earth" );
+			farm.setAccreditations( new HashSet<Farm.Accreditation>() );
+			farm.getAccreditations().add( Farm.Accreditation.ORGANIC );
+			farm.getAccreditations().add( Farm.Accreditation.SUSTAINABLE );
+			s.save( farm );
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// assertion that accreditations collection table got populated
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.doWork(
+					new Work() {
+						@Override
+						public void execute(Connection connection) throws SQLException {
+							final Statement statement = connection.createStatement();
+							final ResultSet resultSet = statement.executeQuery( "select count(*) from farm_accreditations" );
+							assertTrue( resultSet.next() );
+							final int count = resultSet.getInt( 1 );
+							assertEquals( 2, count );
+						}
+					}
+			);
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// do delete
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.createQuery( "delete Farm" ).executeUpdate();
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// assertion that accreditations collection table got cleaned up
+		//		if they didn't, the delete should have caused a constraint error, but just to be sure...
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.doWork(
+					new Work() {
+						@Override
+						public void execute(Connection connection) throws SQLException {
+							final Statement statement = connection.createStatement();
+							final ResultSet resultSet = statement.executeQuery( "select count(*) from farm_accreditations" );
+							assertTrue( resultSet.next() );
+							final int count = resultSet.getInt( 1 );
+							assertEquals( 0, count );
+						}
+					}
+			);
+			s.getTransaction().commit();
+			s.close();
+		}
+
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-9222" )
+	public void testBulkDeleteOfMultiTableEntityWithElementCollection() {
+		// set up test data
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			Human human = new Human();
+			human.setNickNames( new TreeSet() );
+			human.getNickNames().add( "Johnny" );
+			s.save( human );
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// assertion that nickname collection table got populated
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.doWork(
+					new Work() {
+						@Override
+						public void execute(Connection connection) throws SQLException {
+							final Statement statement = connection.createStatement();
+							final ResultSet resultSet = statement.executeQuery( "select count(*) from human_nick_names" );
+							assertTrue( resultSet.next() );
+							final int count = resultSet.getInt( 1 );
+							assertEquals( 1, count );
+						}
+					}
+			);
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// do delete
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.createQuery( "delete Human" ).executeUpdate();
+			s.getTransaction().commit();
+			s.close();
+		}
+
+		// assertion that nickname collection table got cleaned up
+		//		if they didn't, the delete should have caused a constraint error, but just to be sure...
+		{
+			Session s = openSession();
+			s.beginTransaction();
+			s.doWork(
+					new Work() {
+						@Override
+						public void execute(Connection connection) throws SQLException {
+							final Statement statement = connection.createStatement();
+							final ResultSet resultSet = statement.executeQuery( "select count(*) from human_nick_names" );
+							assertTrue( resultSet.next() );
+							final int count = resultSet.getInt( 1 );
+							assertEquals( 0, count );
+						}
+					}
+			);
+			s.getTransaction().commit();
+			s.close();
+		}
+	}
+
+
+
+
 	private class TestData {
 
 		private Animal polliwog;

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

@@ -21,9 +21,12 @@
 package org.hibernate.test.hql;
 
 import java.util.List;
-
+import java.util.Set;
 import javax.persistence.CascadeType;
+import javax.persistence.CollectionTable;
+import javax.persistence.ElementCollection;
 import javax.persistence.Entity;
+import javax.persistence.Enumerated;
 import javax.persistence.GeneratedValue;
 import javax.persistence.Id;
 import javax.persistence.ManyToMany;
@@ -38,10 +41,15 @@ public class Farm {
 	private long id;
 	
 	private String name;
-	
+
 	@ManyToMany(cascade = CascadeType.ALL)
 	private List<Crop> crops;
 
+	@ElementCollection
+	@Enumerated
+	@CollectionTable( name = "farm_accreditations" )
+	private Set<Accreditation> accreditations;
+
 	public long getId() {
 		return id;
 	}
@@ -65,4 +73,18 @@ public class Farm {
 	public void setCrops(List<Crop> crops) {
 		this.crops = crops;
 	}
+
+	public Set<Accreditation> getAccreditations() {
+		return accreditations;
+	}
+
+	public void setAccreditations(Set<Accreditation> accreditations) {
+		this.accreditations = accreditations;
+	}
+
+	public static enum Accreditation {
+		ORGANIC,
+		SUSTAINABLE,
+		FARM_TO_TABLE
+	}
 }