From d8b58cef67a110914de37eb5dad17c7bbd06ba66 Mon Sep 17 00:00:00 2001 From: Brett Meyer Date: Tue, 17 Sep 2013 13:29:43 -0400 Subject: [PATCH] HHH-8476 Bulk delete doesn't cascade delete on join table --- .../java/org/hibernate/dialect/Dialect.java | 10 ++ .../java/org/hibernate/dialect/H2Dialect.java | 5 + .../hql/internal/ast/QueryTranslatorImpl.java | 15 +-- .../hql/internal/ast/exec/BasicExecutor.java | 11 +- .../hql/internal/ast/exec/DeleteExecutor.java | 119 ++++++++++++++++++ .../hql/spi/TableBasedDeleteHandlerImpl.java | 1 + .../test/hql/BulkManipulationTest.java | 13 ++ 7 files changed, 164 insertions(+), 10 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index 1d8978367a..27bf3516c9 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -2403,4 +2403,14 @@ public abstract class Dialect implements ConversionContext { public String getNotExpression( String expression ) { return "not " + expression; } + + /** + * Does this dialect support tuples in subqueries? Ex: + * delete from Table1 where (col1, col2) in (select col1, col2 from Table2) + * + * @return boolean + */ + public boolean supportsTuplesInSubqueries() { + return true; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java index 288e205985..1b740327fc 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java @@ -390,4 +390,9 @@ public class H2Dialect extends Dialect { // see http://groups.google.com/group/h2-database/browse_thread/thread/562d8a49e2dabe99?hl=en return true; } + + @Override + public boolean supportsTuplesInSubqueries() { + return false; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java index 7ca4debbdd..5fd2377aed 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java @@ -30,12 +30,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import antlr.ANTLRException; -import antlr.RecognitionException; -import antlr.TokenStreamException; -import antlr.collections.AST; -import org.jboss.logging.Logger; - import org.hibernate.HibernateException; import org.hibernate.MappingException; import org.hibernate.QueryException; @@ -50,6 +44,7 @@ import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes; import org.hibernate.hql.internal.antlr.HqlTokenTypes; import org.hibernate.hql.internal.antlr.SqlTokenTypes; import org.hibernate.hql.internal.ast.exec.BasicExecutor; +import org.hibernate.hql.internal.ast.exec.DeleteExecutor; import org.hibernate.hql.internal.ast.exec.MultiTableDeleteExecutor; import org.hibernate.hql.internal.ast.exec.MultiTableUpdateExecutor; import org.hibernate.hql.internal.ast.exec.StatementExecutor; @@ -71,6 +66,12 @@ import org.hibernate.loader.hql.QueryLoader; import org.hibernate.param.ParameterSpecification; import org.hibernate.persister.entity.Queryable; import org.hibernate.type.Type; +import org.jboss.logging.Logger; + +import antlr.ANTLRException; +import antlr.RecognitionException; +import antlr.TokenStreamException; +import antlr.collections.AST; /** * A QueryTranslator that uses an Antlr-based parser. @@ -531,7 +532,7 @@ public class QueryTranslatorImpl implements FilterTranslator { return new MultiTableDeleteExecutor( walker ); } else { - return new BasicExecutor( walker, persister ); + return new DeleteExecutor( walker, persister ); } } else if ( walker.getStatementType() == HqlSqlTokenTypes.UPDATE ) { diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/BasicExecutor.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/BasicExecutor.java index 255dc1989e..918efe0358 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/BasicExecutor.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/BasicExecutor.java @@ -74,6 +74,11 @@ public class BasicExecutor implements StatementExecutor { } public int execute(QueryParameters parameters, SessionImplementor session) throws HibernateException { + return doExecute( parameters, session, sql, parameterSpecifications ); + } + + protected int doExecute(QueryParameters parameters, SessionImplementor session, String sql, + List parameterSpecifications) throws HibernateException { BulkOperationCleanupAction action = new BulkOperationCleanupAction( session, persister ); if ( session.isEventSource() ) { ( (EventSource) session ).getActionQueue().addAction( action ); @@ -88,10 +93,10 @@ public class BasicExecutor implements StatementExecutor { try { try { st = session.getTransactionCoordinator().getJdbcCoordinator().getStatementPreparer().prepareStatement( sql, false ); - Iterator parameterSpecifications = this.parameterSpecifications.iterator(); + Iterator paramSpecItr = parameterSpecifications.iterator(); int pos = 1; - while ( parameterSpecifications.hasNext() ) { - final ParameterSpecification paramSpec = ( ParameterSpecification ) parameterSpecifications.next(); + while ( paramSpecItr.hasNext() ) { + final ParameterSpecification paramSpec = (ParameterSpecification) paramSpecItr.next(); pos += paramSpec.bind( st, parameters, session, pos ); } if ( selection != null ) { 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 new file mode 100644 index 0000000000..daf8d18d5e --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java @@ -0,0 +1,119 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * JBoss, Home of Professional Open Source + * Copyright 2013 Red Hat Inc. and/or its affiliates and other contributors + * as indicated by the @authors tag. All rights reserved. + * See the copyright.txt in the distribution for a + * full listing of individual contributors. + * + * 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, v. 2.1. + * This program is distributed in the hope that it will be useful, but WITHOUT A + * 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, + * v.2.1 along with this distribution; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ +package org.hibernate.hql.internal.ast.exec; + +import java.util.ArrayList; +import java.util.List; + +import org.hibernate.HibernateException; +import org.hibernate.dialect.Dialect; +import org.hibernate.engine.spi.QueryParameters; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.engine.spi.SessionImplementor; +import org.hibernate.hql.internal.ast.HqlSqlWalker; +import org.hibernate.hql.internal.ast.SqlGenerator; +import org.hibernate.hql.internal.ast.tree.DeleteStatement; +import org.hibernate.internal.util.StringHelper; +import org.hibernate.param.ParameterSpecification; +import org.hibernate.persister.collection.AbstractCollectionPersister; +import org.hibernate.persister.entity.Queryable; +import org.hibernate.sql.Delete; +import org.hibernate.type.CollectionType; +import org.hibernate.type.Type; +import org.jboss.logging.Logger; + +import antlr.RecognitionException; +import antlr.collections.AST; + + +/** + * Provides deletions in addition to the basic SQL delete statement being executed. Ex: cascading the delete into a + * many-to-many join table. + * + * @author Brett Meyer + */ +public class DeleteExecutor extends BasicExecutor { + + private static final Logger LOG = Logger.getLogger( DeleteExecutor.class ); + + private final List deletes = new ArrayList(); + + private List parameterSpecifications; + + public DeleteExecutor(HqlSqlWalker walker, Queryable persister) { + super( walker, persister ); + + final SessionFactoryImplementor factory = walker.getSessionFactoryHelper().getFactory(); + final Dialect dialect = factory.getDialect(); + + final DeleteStatement deleteStatement = ( DeleteStatement ) walker.getAST(); + final AST whereClause = deleteStatement.getWhereClause(); + + try { + final SqlGenerator gen = new SqlGenerator( factory ); + gen.whereClause( whereClause ); + parameterSpecifications = gen.getCollectedParameters(); + + // If many-to-many, delete the FK row in the collection table. + 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 idSubselectWhere = gen.getSQL().length() > 7 ? gen.getSQL() : ""; + 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() ); + } + } + } + } + } + catch (RecognitionException e) { + throw new HibernateException( "Unable to delete the FKs in the join table!", e ); + } + } + + @Override + public int execute(QueryParameters parameters, SessionImplementor session) throws HibernateException { + for (String delete : deletes) { + doExecute( parameters, session, delete, parameterSpecifications ); + } + + // finally, execute the original sql statement + return super.execute( parameters, session ); + } +} 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 73b68866ed..639fe6cbf6 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,6 +83,7 @@ public class TableBasedDeleteHandlerImpl 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. for ( Type type : targetedPersister.getPropertyTypes() ) { if ( type.isCollectionType() ) { CollectionType cType = (CollectionType) type; 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 28b9e6ef2a..779c5cdfa3 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 @@ -30,7 +30,9 @@ import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import junit.framework.AssertionFailedError; @@ -1208,7 +1210,18 @@ public class BulkManipulationTest extends BaseCoreFunctionalTestCase { s.flush(); + Zoo zoo = new Zoo(); + Map directors = new HashMap(); + directors.put( brett.getId().toString(), brett ); + zoo.setDirectors( directors ); + s.save( zoo ); + + s.flush(); + try { + // non-multitable + s.createQuery( "delete from Zoo" ).executeUpdate(); + // multitable (joined subclass) s.createQuery( "delete from Human" ).executeUpdate(); } catch (ConstraintViolationException cve) {