From 873f28f5e67f179248017d0e13fceaaeb92d4b14 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 9 Jul 2014 22:45:00 -0700 Subject: [PATCH] HHH-9282 : Revert HHH-9222 on master --- .../hql/internal/ast/exec/DeleteExecutor.java | 63 ++++++++----------- .../hql/spi/TableBasedDeleteHandlerImpl.java | 42 +++---------- .../test/hql/BulkManipulationTest.java | 5 +- 3 files changed, 38 insertions(+), 72 deletions(-) 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 eef31597d7..7949f16c8c 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 @@ -78,49 +78,36 @@ public class DeleteExecutor extends BasicExecutor { parameterSpecifications = new ArrayList(); idSubselectWhere = ""; } - - // find plural attributes defined for the entity being deleted... + + // If many-to-many, delete the FK row in the collection table. for ( Type type : persister.getPropertyTypes() ) { - 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 + if ( type.isCollectionType() ) { + 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" + + 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." - ); - continue; + ); + } + 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() ); + } + } } - - 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 3d93041fed..8e890fb492 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 @@ -83,39 +83,17 @@ public class TableBasedDeleteHandlerImpl final String idSubselect = generateIdSubselect( targetedPersister ); deletes = new ArrayList(); - // 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. + // 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. for ( Type type : targetedPersister.getPropertyTypes() ) { - if ( ! type.isCollectionType() ) { - continue; + 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")); + } } - - 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(); @@ -125,7 +103,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 47e26b36f4..be1c69a369 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 @@ -46,6 +46,7 @@ import org.hibernate.jdbc.Work; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.testing.DialectChecks; +import org.hibernate.testing.FailureExpected; import org.hibernate.testing.FailureExpectedWithNewMetamodel; import org.hibernate.testing.FailureExpectedWithNewUnifiedXsd; import org.hibernate.testing.RequiresDialectFeature; @@ -1426,7 +1427,7 @@ public class BulkManipulationTest extends BaseCoreFunctionalTestCase { } @Test - @TestForIssue( jiraKey = "HHH-9222" ) + @FailureExpected( jiraKey = "HHH-9282", message = "failed because HHH-9222 was reverted by HHH-9282") public void testBulkDeleteOfEntityWithElementCollection() { // set up test data { @@ -1495,7 +1496,7 @@ public class BulkManipulationTest extends BaseCoreFunctionalTestCase { } @Test - @TestForIssue( jiraKey = "HHH-9222" ) + @FailureExpected( jiraKey = "HHH-9282", message = "failed because HHH-9222 was reverted by HHH-9282") public void testBulkDeleteOfMultiTableEntityWithElementCollection() { // set up test data {