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

This reverts commit 58dd469e4c.
This commit is contained in:
Andrea Boriero 2019-12-05 12:34:27 +00:00
parent e924d55fdf
commit 963a516ea8
8 changed files with 11 additions and 152 deletions

View File

@ -7,7 +7,6 @@
package org.hibernate.hql.internal.ast.exec;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.hibernate.HibernateException;
@ -24,7 +23,6 @@ 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;
@ -60,8 +58,7 @@ public class DeleteExecutor extends BasicExecutor {
final SqlGenerator gen = new SqlGenerator( factory );
gen.whereClause( whereClause );
parameterSpecifications = gen.getCollectedParameters();
String sql = gen.getSQL();
idSubselectWhere = sql.length() > 7 ? sql : "";
idSubselectWhere = gen.getSQL().length() > 7 ? gen.getSQL() : "";
}
else {
parameterSpecifications = new ArrayList<>();
@ -77,20 +74,8 @@ public class DeleteExecutor extends BasicExecutor {
final CollectionType cType = (CollectionType) type;
final AbstractCollectionPersister cPersister = (AbstractCollectionPersister) metamodel.collectionPersister( cType.getRole() );
if ( cPersister.isManyToMany() ) {
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 ) {
if ( persister.getIdentifierColumnNames().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" +
@ -98,13 +83,11 @@ public class DeleteExecutor extends BasicExecutor {
);
}
else {
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 String idSubselect = "(select "
+ String.join( ", ", persister.getIdentifierColumnNames() ) + " from "
+ persister.getTableName() + idSubselectWhere + ")";
final String where = "(" + String.join( ", ", cPersister.getKeyColumnNames() )
+ ") in " + idSubselect;
final Delete delete = new Delete().setTableName( cPersister.getTableName() ).setWhere( where );
if ( commentsEnabled ) {
delete.setComment( "delete FKs in join table" );
@ -119,14 +102,6 @@ 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,7 +6,6 @@
*/
package org.hibernate.hql.spi.id;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -17,7 +16,6 @@ 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;
@ -25,8 +23,6 @@ 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}
@ -197,40 +193,6 @@ 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(), generateIdSubselect( targetedPersister, cPersister, idTableInfo ), "bulk delete - m2m join table cleanup"));
cPersister.getKeyColumnNames(), idSubselect, "bulk delete - m2m join table cleanup"));
}
}
}

View File

@ -17,11 +17,8 @@ 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.
*
@ -91,32 +88,9 @@ 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(),
generateIdSubselect( idSubselect, getTargetedQueryable(), cPersister ),
idSubselect,
"bulk delete - m2m join table cleanup"
) );
}

View File

@ -9,7 +9,6 @@ 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;
@ -18,7 +17,6 @@ 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;
@ -63,7 +61,7 @@ public abstract class AbstractInlineIdsDeleteHandlerImpl
deletes.add( generateDelete(
cPersister.getTableName(),
cPersister.getKeyColumnNames(),
generateIdSubselect( idSubselect, getTargetedQueryable(), cPersister ),
idSubselect,
"bulk delete - m2m join table cleanup"
).toStatementString() );
}
@ -104,28 +102,6 @@ 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,7 +15,6 @@ 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;
@ -45,11 +44,6 @@ 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,7 +11,6 @@ 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;
@ -22,13 +21,10 @@ 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;
@ -243,16 +239,6 @@ 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();
@ -330,14 +316,6 @@ 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")