HHH-13752 - Test and fix deletion of entities with many-to-many assocations using non-primary keys for join table

This commit is contained in:
Christian Beikov 2019-11-26 16:25:10 +01:00 committed by Andrea Boriero
parent a133aff97f
commit 58dd469e4c
8 changed files with 152 additions and 11 deletions

View File

@ -7,6 +7,7 @@
package org.hibernate.hql.internal.ast.exec;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.hibernate.HibernateException;
@ -23,6 +24,7 @@ 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.ComponentType;
import org.hibernate.type.Type;
import org.jboss.logging.Logger;
@ -58,7 +60,8 @@ public class DeleteExecutor extends BasicExecutor {
final SqlGenerator gen = new SqlGenerator( factory );
gen.whereClause( whereClause );
parameterSpecifications = gen.getCollectedParameters();
idSubselectWhere = gen.getSQL().length() > 7 ? gen.getSQL() : "";
String sql = gen.getSQL();
idSubselectWhere = sql.length() > 7 ? sql : "";
}
else {
parameterSpecifications = new ArrayList<>();
@ -74,8 +77,20 @@ public class DeleteExecutor extends BasicExecutor {
final CollectionType cType = (CollectionType) type;
final AbstractCollectionPersister cPersister = (AbstractCollectionPersister) metamodel.collectionPersister( cType.getRole() );
if ( cPersister.isManyToMany() ) {
if ( persister.getIdentifierColumnNames().length > 1
&& notSupportingTuplesInSubqueries ) {
Type keyType = cPersister.getKeyType();
String[] columnNames;
if ( keyType.isComponentType() ) {
ComponentType componentType = (ComponentType) keyType;
List<String> columns = new ArrayList<>( componentType.getPropertyNames().length );
for ( String propertyName : componentType.getPropertyNames() ) {
Collections.addAll( columns, persister.toColumns( propertyName ) );
}
columnNames = columns.toArray( new String[columns.size()] );
}
else {
columnNames = persister.getIdentifierColumnNames();
}
if ( columnNames.length > 1 && notSupportingTuplesInSubqueries ) {
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" +
@ -83,11 +98,13 @@ public class DeleteExecutor extends BasicExecutor {
);
}
else {
final String idSubselect = "(select "
+ String.join( ", ", persister.getIdentifierColumnNames() ) + " from "
+ persister.getTableName() + idSubselectWhere + ")";
final String where = "(" + String.join( ", ", cPersister.getKeyColumnNames() )
+ ") in " + idSubselect;
StringBuilder whereBuilder = new StringBuilder();
whereBuilder.append( '(' );
append( ", ", cPersister.getKeyColumnNames(), whereBuilder );
whereBuilder.append( ") in (select " );
append( ", ", columnNames, whereBuilder );
final String where = whereBuilder.append(" from ")
.append( persister.getTableName() ).append( idSubselectWhere ).append( ")" ).toString();
final Delete delete = new Delete().setTableName( cPersister.getTableName() ).setWhere( where );
if ( commentsEnabled ) {
delete.setComment( "delete FKs in join table" );
@ -102,6 +119,14 @@ public class DeleteExecutor extends BasicExecutor {
throw new HibernateException( "Unable to delete the FKs in the join table!", e );
}
}
private static void append(String delimiter, String[] parts, StringBuilder sb) {
sb.append( parts[0] );
for ( int i = 1; i < parts.length; i++ ) {
sb.append( delimiter );
sb.append( parts[i] );
}
}
@Override
public int execute(QueryParameters parameters, SharedSessionContractImplementor session) throws HibernateException {

View File

@ -6,6 +6,7 @@
*/
package org.hibernate.hql.spi.id;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -16,6 +17,7 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.internal.ast.SqlGenerator;
import org.hibernate.param.ParameterSpecification;
import org.hibernate.persister.collection.AbstractCollectionPersister;
import org.hibernate.persister.entity.Queryable;
import org.hibernate.sql.InsertSelect;
import org.hibernate.sql.Select;
@ -23,6 +25,8 @@ import org.hibernate.sql.SelectValues;
import antlr.RecognitionException;
import antlr.collections.AST;
import org.hibernate.type.ComponentType;
import org.hibernate.type.Type;
/**
* Convenience base class for {@link MultiTableBulkIdStrategy.UpdateHandler}
@ -193,6 +197,40 @@ public abstract class AbstractTableBasedBulkIdHandler {
" from " + idTableInfo.getQualifiedIdTableName();
}
protected String generateIdSubselect(Queryable persister, AbstractCollectionPersister cPersister, IdTableInfo idTableInfo) {
String[] columnNames = getKeyColumnNames( persister, cPersister );
StringBuilder selectBuilder = new StringBuilder();
selectBuilder.append( "select " );
appendJoined( ", ", columnNames, selectBuilder );
return selectBuilder.append( " from " )
.append( idTableInfo.getQualifiedIdTableName() ).toString();
}
protected static String[] getKeyColumnNames(Queryable persister, AbstractCollectionPersister cPersister) {
Type keyType = cPersister.getKeyType();
String[] columnNames;
if ( keyType.isComponentType() ) {
ComponentType componentType = (ComponentType) keyType;
List<String> columns = new ArrayList<>(componentType.getPropertyNames().length );
for ( String propertyName : componentType.getPropertyNames() ) {
Collections.addAll( columns, persister.toColumns( propertyName ) );
}
columnNames = columns.toArray( new String[columns.size()] );
}
else {
columnNames = persister.getIdentifierColumnNames();
}
return columnNames;
}
protected static void appendJoined(String delimiter, String[] parts, StringBuilder sb) {
sb.append( parts[0] );
for ( int i = 1; i < parts.length; i++ ) {
sb.append( delimiter );
sb.append( parts[i] );
}
}
protected void prepareForUse(Queryable persister, SharedSessionContractImplementor session) {
}

View File

@ -68,7 +68,7 @@ public class TableBasedDeleteHandlerImpl
AbstractCollectionPersister cPersister = (AbstractCollectionPersister) factory.getMetamodel().collectionPersister( cType.getRole() );
if ( cPersister.isManyToMany() ) {
deletes.add( generateDelete( cPersister.getTableName(),
cPersister.getKeyColumnNames(), idSubselect, "bulk delete - m2m join table cleanup"));
cPersister.getKeyColumnNames(), generateIdSubselect( targetedPersister, cPersister, idTableInfo ), "bulk delete - m2m join table cleanup"));
}
}
}

View File

@ -17,8 +17,11 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.spi.id.AbstractIdsBulkIdHandler;
import org.hibernate.internal.util.StringHelper;
import org.hibernate.persister.collection.AbstractCollectionPersister;
import org.hibernate.persister.entity.Queryable;
import java.util.Arrays;
/**
* Defines how identifier values are selected from the updatable/deletable tables.
*
@ -88,9 +91,32 @@ public abstract class AbstractCteValuesListBulkIdHandler extends
) )
.append( " from " )
.append( determineIdTableName( persister ) )
.append( " tmp2" )
.toString();
}
protected String generateIdSubselect(String idSubselect, Queryable persister, AbstractCollectionPersister cPersister) {
String[] columnNames = getKeyColumnNames( persister, cPersister );
// If the column names are equal to the identifier column names, just return the idSubselect
if ( Arrays.equals( getTargetedQueryable().getIdentifierColumnNames(), columnNames ) ) {
return idSubselect;
}
// Otherwise, we need to fetch the key column names from the original table
// Unfortunately, this is a bit more work, as only the identifiers are fetched
// It would be great if we could adapt #selectIds to fetch key columns as well
StringBuilder selectBuilder = new StringBuilder();
selectBuilder.append( "select " );
appendJoined( ", ", columnNames, selectBuilder );
selectBuilder.append( " from " ).append( getTargetedQueryable().getTableName() );
selectBuilder.append( " tmp where (" );
appendJoined( ", ", getTargetedQueryable().getIdentifierColumnNames(), selectBuilder );
selectBuilder.append( ") in (select " );
appendJoined( ", ", getTargetedQueryable().getIdentifierColumnNames(), selectBuilder );
selectBuilder.append( " from " ).append( determineIdTableName( persister ) ).append( " tmp2)" );
return selectBuilder.toString();
}
protected CteValuesListBuilder prepareCteStatement(
SharedSessionContractImplementor session,
QueryParameters queryParameters) {

View File

@ -56,7 +56,7 @@ public class CteValuesListDeleteHandlerImpl
deletes.add( generateDelete(
cPersister.getTableName(),
cPersister.getKeyColumnNames(),
idSubselect,
generateIdSubselect( idSubselect, getTargetedQueryable(), cPersister ),
"bulk delete - m2m join table cleanup"
) );
}

View File

@ -9,6 +9,7 @@ package org.hibernate.hql.spi.id.inline;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.hibernate.engine.spi.QueryParameters;
@ -17,6 +18,7 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.spi.id.MultiTableBulkIdStrategy;
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;
@ -61,7 +63,7 @@ public abstract class AbstractInlineIdsDeleteHandlerImpl
deletes.add( generateDelete(
cPersister.getTableName(),
cPersister.getKeyColumnNames(),
idSubselect,
generateIdSubselect( idSubselect, getTargetedQueryable(), cPersister ),
"bulk delete - m2m join table cleanup"
).toStatementString() );
}
@ -102,6 +104,28 @@ public abstract class AbstractInlineIdsDeleteHandlerImpl
return values.getIds().size();
}
protected String generateIdSubselect(String idSubselect, Queryable persister, AbstractCollectionPersister cPersister) {
String[] columnNames = getKeyColumnNames( persister, cPersister );
// If the column names are equal to the identifier column names, just return the idSubselect
if ( Arrays.equals(getTargetedQueryable().getIdentifierColumnNames(), columnNames ) ) {
return idSubselect;
}
// Otherwise, we need to fetch the key column names from the original table
// Unfortunately, this is a bit more work, as only the identifiers are fetched
// It would be great if we could adapt #selectIds to fetch key columns as well
StringBuilder selectBuilder = new StringBuilder();
selectBuilder.append( "select " );
appendJoined( ", ", columnNames, selectBuilder );
selectBuilder.append( " from " ).append( getTargetedQueryable().getTableName() );
selectBuilder.append( " tmp where (" );
appendJoined( ", ", getTargetedQueryable().getIdentifierColumnNames(), selectBuilder );
selectBuilder.append( ") in (" );
selectBuilder.append( idSubselect );
selectBuilder.append( ")" );
return selectBuilder.toString();
}
protected Delete generateDelete(
String tableName,
String[] columnNames,

View File

@ -15,6 +15,7 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.spi.id.IdTableInfo;
import org.hibernate.hql.spi.id.TableBasedDeleteHandlerImpl;
import org.hibernate.persister.collection.AbstractCollectionPersister;
import org.hibernate.persister.entity.Queryable;
import org.hibernate.sql.SelectValues;
@ -44,6 +45,11 @@ public class DeleteHandlerImpl extends TableBasedDeleteHandlerImpl {
return super.generateIdSubselect( persister, idTableInfo ) + " where " + SESSION_ID_COLUMN_NAME + "=?";
}
@Override
protected String generateIdSubselect(Queryable persister, AbstractCollectionPersister cPersister, IdTableInfo idTableInfo) {
return super.generateIdSubselect( persister, cPersister, idTableInfo ) + " where " + SESSION_ID_COLUMN_NAME + "=?";
}
@Override
protected int handlePrependedParametersOnIdSelection(PreparedStatement ps, SharedSessionContractImplementor session, int pos) throws SQLException {
Helper.INSTANCE.bindSessionIdentifier( ps, session, pos );

View File

@ -11,6 +11,7 @@ import org.hibernate.annotations.NaturalId;
import org.hibernate.hql.spi.FilterTranslator;
import org.hibernate.query.Query;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import org.junit.Before;
import org.junit.Test;
@ -21,10 +22,13 @@ import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.JoinTable;
import javax.persistence.ManyToOne;
import javax.persistence.OneToMany;
import javax.persistence.Table;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
import static org.junit.Assert.assertEquals;
@ -239,6 +243,16 @@ public class NaturalIdDereferenceTest extends BaseCoreFunctionalTestCase {
} );
}
@Test
@TestForIssue(jiraKey = "HHH-13752")
public void deleteWithNaturalIdBasedJoinTable() {
doInHibernate( this::sessionFactory, session -> {
Query query = session.createQuery(
"DELETE FROM Book b WHERE 1=0" );
query.executeUpdate();
} );
}
private int getSQLJoinCount(Query query) {
String sqlQuery = getSQLQuery( query ).toLowerCase();
@ -316,6 +330,14 @@ public class NaturalIdDereferenceTest extends BaseCoreFunctionalTestCase {
@Column(name = "isbn", unique = true)
private String isbn;
@OneToMany
@JoinTable(
name = "similar_books",
joinColumns = @JoinColumn(name = "base_isbn", referencedColumnName = "isbn"),
inverseJoinColumns = @JoinColumn(name = "ref_isbn", referencedColumnName = "isbn_ref")
)
private Set<BookRef> similarBooks;
}
@Entity(name = "BookRef")