diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java index e7752470ff..63384f4179 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java +++ b/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(); 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() ) { - final CollectionType cType = (CollectionType) type; - 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 ( ! type.isCollectionType() ) { + continue; } + + // 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) { diff --git a/hibernate-core/src/main/java/org/hibernate/hql/spi/TableBasedDeleteHandlerImpl.java b/hibernate-core/src/main/java/org/hibernate/hql/spi/TableBasedDeleteHandlerImpl.java index 639fe6cbf6..75f93511fa 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/spi/TableBasedDeleteHandlerImpl.java +++ b/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(); - // If many-to-many, delete the FK row in the collection table. - // This partially overlaps with DeleteExecutor, but it instead uses the temp table in the idSubselect. + // find plural attributes defined for the entity being deleted... + // + // NOTE : this partially overlaps with DeleteExecutor, but it instead + // uses the temp table in the idSubselect. for ( Type type : targetedPersister.getPropertyTypes() ) { - if ( type.isCollectionType() ) { - CollectionType cType = (CollectionType) type; - AbstractCollectionPersister cPersister = (AbstractCollectionPersister)factory.getCollectionPersister( cType.getRole() ); - if ( cPersister.isManyToMany() ) { - deletes.add( generateDelete( cPersister.getTableName(), - cPersister.getKeyColumnNames(), idSubselect, "bulk delete - m2m join table cleanup")); - } + if ( ! type.isCollectionType() ) { + continue; } + + 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" ) ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/BulkManipulationTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/BulkManipulationTest.java index 22f9031b7f..3d7bb39842 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/hql/BulkManipulationTest.java +++ b/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 static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; - -import junit.framework.AssertionFailedError; +import java.util.TreeSet; 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.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; diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/Farm.java b/hibernate-core/src/test/java/org/hibernate/test/hql/Farm.java index a3abd13373..e75de30f1e 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/hql/Farm.java +++ b/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 crops; + @ElementCollection + @Enumerated + @CollectionTable( name = "farm_accreditations" ) + private Set accreditations; + public long getId() { return id; } @@ -65,4 +73,18 @@ public class Farm { public void setCrops(List crops) { this.crops = crops; } + + public Set getAccreditations() { + return accreditations; + } + + public void setAccreditations(Set accreditations) { + this.accreditations = accreditations; + } + + public static enum Accreditation { + ORGANIC, + SUSTAINABLE, + FARM_TO_TABLE + } }