Skip to content

HHH-17167 Unable to locate parameter for RESTRICT - DELETE error when removing entity with RowId #7306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 15, 2023
Merged

HHH-17167 Unable to locate parameter for RESTRICT - DELETE error when removing entity with RowId #7306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d36c13
HHH-17167 Add test for issue
mbladel Sep 7, 2023
2b73799
HHH-17167 Also use `@RowId` for deletes when available
mbladel Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hibernate-core/src/main/java/org/hibernate/annotations/RowId.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

/**
* Specifies that a {@code rowid}-like column or pseudo-column should be
* used as the row locator in SQL {@code update} statements for an entity,
* used as the row locator in CRUD operations for an entity,
* instead of the primary key of the table.
* <p>
* If the {@linkplain org.hibernate.dialect.Dialect SQL dialect} does
Expand Down
48 changes: 48 additions & 0 deletions ...re/src/main/java/org/hibernate/persister/entity/mutation/AbstractMutationCoordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.hibernate.sql.model.ast.TableMutation;
import org.hibernate.sql.model.ast.builder.ColumnValuesTableMutationBuilder;
import org.hibernate.sql.model.ast.builder.MutationGroupBuilder;
import org.hibernate.sql.model.ast.builder.RestrictedTableMutationBuilder;
import org.hibernate.sql.model.internal.MutationOperationGroupFactory;

/**
Expand Down Expand Up @@ -165,4 +166,51 @@ protected void bindPartitionColumnValueBindings(
}
}
}

protected static boolean needsRowId(AbstractEntityPersister entityPersister, EntityTableMapping tableMapping) {
return entityPersister.getRowIdMapping() != null && tableMapping.isIdentifierTable();
}

protected static void applyKeyRestriction(
Object rowId,
AbstractEntityPersister entityPersister,
RestrictedTableMutationBuilder<?, ?> tableMutationBuilder,
EntityTableMapping tableMapping) {
if ( rowId != null && needsRowId( entityPersister, tableMapping ) ) {
tableMutationBuilder.addKeyRestrictionLeniently( entityPersister.getRowIdMapping() );
}
else {
tableMutationBuilder.addKeyRestrictions( tableMapping.getKeyMapping() );
}
}

protected void breakDownKeyJdbcValues(
Object id,
Object rowId,
SharedSessionContractImplementor session,
JdbcValueBindings jdbcValueBindings,
EntityTableMapping tableMapping) {
if ( rowId != null && needsRowId( entityPersister(), tableMapping ) ) {
jdbcValueBindings.bindValue(
rowId,
tableMapping.getTableName(),
entityPersister().getRowIdMapping().getRowIdName(),
ParameterUsage.RESTRICT
);
}
else {
tableMapping.getKeyMapping().breakDownKeyJdbcValues(
id,
(jdbcValue, columnMapping) -> {
jdbcValueBindings.bindValue(
jdbcValue,
tableMapping.getTableName(),
columnMapping.getColumnName(),
ParameterUsage.RESTRICT
);
},
session
);
}
}
}
71 changes: 16 additions & 55 deletions hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,18 @@
import org.hibernate.engine.jdbc.mutation.MutationExecutor;
import org.hibernate.engine.jdbc.mutation.ParameterUsage;
import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails;
import org.hibernate.engine.jdbc.mutation.spi.MutationExecutorService;
import org.hibernate.engine.spi.EntityEntry;
import org.hibernate.engine.spi.PersistenceContext;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.metamodel.mapping.AttributeMapping;
import org.hibernate.metamodel.mapping.AttributeMappingsList;
import org.hibernate.metamodel.mapping.EntityRowIdMapping;
import org.hibernate.metamodel.mapping.EntityVersionMapping;
import org.hibernate.metamodel.mapping.SelectableMapping;
import org.hibernate.persister.entity.AbstractEntityPersister;
import org.hibernate.sql.model.MutationOperation;
import org.hibernate.sql.model.MutationOperationGroup;
import org.hibernate.sql.model.MutationType;
import org.hibernate.sql.model.ast.ColumnValueBinding;
import org.hibernate.sql.model.ast.ColumnValueBindingList;
import org.hibernate.sql.model.ast.builder.MutationGroupBuilder;
import org.hibernate.sql.model.ast.builder.RestrictedTableMutationBuilder;
Expand All @@ -52,7 +49,7 @@ public class DeleteCoordinator extends AbstractMutationCoordinator {
public DeleteCoordinator(AbstractEntityPersister entityPersister, SessionFactoryImplementor factory) {
super( entityPersister, factory );

this.staticOperationGroup = generateOperationGroup( null, true, null );
this.staticOperationGroup = generateOperationGroup( "", null, true, null );
this.batchKey = new BasicBatchKey( entityPersister.getEntityName() + "#DELETE" );

if ( !entityPersister.isVersioned() ) {
Expand All @@ -79,24 +76,23 @@ public void coordinateDelete(

final PersistenceContext persistenceContext = session.getPersistenceContextInternal();
final EntityEntry entry = persistenceContext.getEntry( entity );
final Object[] loadedState = entry != null && isImpliedOptimisticLocking ? entry.getLoadedState() : null;
final Object rowId = entry != null && entityPersister().hasRowId() ? entry.getRowId() : null;
final Object[] loadedState = entry != null ? entry.getLoadedState() : null;
final Object rowId = entry != null ? entry.getRowId() : null;

if ( ( isImpliedOptimisticLocking && loadedState != null ) || rowId != null ) {
doDynamicDelete( entity, id, rowId, loadedState, session );
if ( ( isImpliedOptimisticLocking && loadedState != null ) || ( rowId == null && entityPersister().hasRowId() ) ) {
doDynamicDelete( entity, id, loadedState, session );
}
else {
doStaticDelete( entity, id, entry == null ? null : entry.getLoadedState(), version, session );
doStaticDelete( entity, id, rowId, loadedState, version, session );
}
}

protected void doDynamicDelete(
Object entity,
Object id,
Object rowId,
Object[] loadedState,
SharedSessionContractImplementor session) {
final MutationOperationGroup operationGroup = generateOperationGroup( loadedState, true, session );
final MutationOperationGroup operationGroup = generateOperationGroup( null, loadedState, true, session );

final MutationExecutor mutationExecutor = executor( session, operationGroup );

Expand All @@ -110,7 +106,7 @@ protected void doDynamicDelete(

applyLocking( null, loadedState, mutationExecutor, session );

applyId( id, rowId, mutationExecutor, operationGroup, session );
applyId( id, null, mutationExecutor, operationGroup, session );

try {
mutationExecutor.execute(
Expand Down Expand Up @@ -217,14 +213,11 @@ protected void applyId(
MutationExecutor mutationExecutor,
MutationOperationGroup operationGroup,
SharedSessionContractImplementor session) {

final JdbcValueBindings jdbcValueBindings = mutationExecutor.getJdbcValueBindings();
final EntityRowIdMapping rowIdMapping = entityPersister().getRowIdMapping();

for ( int position = 0; position < operationGroup.getNumberOfOperations(); position++ ) {
final MutationOperation jdbcMutation = operationGroup.getOperation( position );
final EntityTableMapping tableDetails = (EntityTableMapping) jdbcMutation.getTableDetails();
breakDownIdJdbcValues( id, rowId, session, jdbcValueBindings, rowIdMapping, tableDetails );
breakDownKeyJdbcValues( id, rowId, session, jdbcValueBindings, tableDetails );
final PreparedStatementDetails statementDetails = mutationExecutor.getPreparedStatementDetails( tableDetails.getTableName() );
if ( statementDetails != null ) {
// force creation of the PreparedStatement
Expand All @@ -234,40 +227,10 @@ protected void applyId(
}
}

private static void breakDownIdJdbcValues(
Object id,
Object rowId,
SharedSessionContractImplementor session,
JdbcValueBindings jdbcValueBindings,
EntityRowIdMapping rowIdMapping,
EntityTableMapping tableDetails) {
if ( rowId != null && rowIdMapping != null && tableDetails.isIdentifierTable() ) {
jdbcValueBindings.bindValue(
rowId,
tableDetails.getTableName(),
rowIdMapping.getRowIdName(),
ParameterUsage.RESTRICT
);
}
else {
tableDetails.getKeyMapping().breakDownKeyJdbcValues(
id,
(jdbcValue, columnMapping) -> {
jdbcValueBindings.bindValue(
jdbcValue,
tableDetails.getTableName(),
columnMapping.getColumnName(),
ParameterUsage.RESTRICT
);
},
session
);
}
}

protected void doStaticDelete(
Object entity,
Object id,
Object rowId,
Object[] loadedState,
Object version,
SharedSessionContractImplementor session) {
Expand Down Expand Up @@ -299,7 +262,7 @@ protected void doStaticDelete(

bindPartitionColumnValueBindings( loadedState, session, jdbcValueBindings );

applyId( id, null, mutationExecutor, staticOperationGroup, session );
applyId( id, rowId, mutationExecutor, staticOperationGroup, session );

mutationExecutor.execute(
entity,
Expand All @@ -321,13 +284,14 @@ protected void doStaticDelete(

protected MutationOperationGroup resolveNoVersionDeleteGroup(SharedSessionContractImplementor session) {
if ( noVersionDeleteGroup == null ) {
noVersionDeleteGroup = generateOperationGroup( null, false, session );
noVersionDeleteGroup = generateOperationGroup( "", null, false, session );
}

return noVersionDeleteGroup;
}

protected MutationOperationGroup generateOperationGroup(
Object rowId,
Object[] loadedState,
boolean applyVersion,
SharedSessionContractImplementor session) {
Expand All @@ -340,21 +304,22 @@ protected MutationOperationGroup generateOperationGroup(
deleteGroupBuilder.addTableDetailsBuilder( tableDeleteBuilder );
} );

applyTableDeleteDetails( deleteGroupBuilder, loadedState, applyVersion, session );
applyTableDeleteDetails( deleteGroupBuilder, rowId, loadedState, applyVersion, session );

return createOperationGroup( null, deleteGroupBuilder.buildMutationGroup() );
}

private void applyTableDeleteDetails(
MutationGroupBuilder deleteGroupBuilder,
Object rowId,
Object[] loadedState,
boolean applyVersion,
SharedSessionContractImplementor session) {
// first, the table key column(s)
deleteGroupBuilder.forEachTableMutationBuilder( (builder) -> {
final EntityTableMapping tableMapping = (EntityTableMapping) builder.getMutatingTable().getTableMapping();
final TableDeleteBuilder tableDeleteBuilder = (TableDeleteBuilder) builder;
applyKeyDetails( tableDeleteBuilder, tableMapping );
applyKeyRestriction( rowId, entityPersister(), tableDeleteBuilder, tableMapping );
} );

if ( applyVersion ) {
Expand Down Expand Up @@ -382,10 +347,6 @@ private void applyTableDeleteDetails(
// todo (6.2) : apply where + where-fragments
}

private static void applyKeyDetails(TableDeleteBuilder tableDeleteBuilder, EntityTableMapping tableMapping) {
tableDeleteBuilder.addKeyRestrictions( tableMapping.getKeyMapping() );
}

protected void applyOptimisticLocking(
MutationGroupBuilder mutationGroupBuilder,
Object[] loadedState,
Expand Down
55 changes: 3 additions & 52 deletions ...core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
package org.hibernate.persister.entity.mutation;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import org.hibernate.AssertionFailure;
import org.hibernate.HibernateException;
Expand All @@ -22,7 +20,6 @@
import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
import org.hibernate.engine.jdbc.mutation.MutationExecutor;
import org.hibernate.engine.jdbc.mutation.ParameterUsage;
import org.hibernate.sql.model.internal.MutationOperationGroupFactory;
import org.hibernate.engine.jdbc.mutation.internal.MutationQueryOptions;
import org.hibernate.engine.jdbc.mutation.internal.NoBatchKeyAccess;
import org.hibernate.engine.jdbc.mutation.spi.BatchKeyAccess;
Expand All @@ -37,7 +34,6 @@
import org.hibernate.internal.util.collections.CollectionHelper;
import org.hibernate.metamodel.mapping.AttributeMapping;
import org.hibernate.metamodel.mapping.AttributeMappingsList;
import org.hibernate.metamodel.mapping.EntityRowIdMapping;
import org.hibernate.metamodel.mapping.EntityVersionMapping;
import org.hibernate.metamodel.mapping.SelectableMapping;
import org.hibernate.metamodel.mapping.SingularAttributeMapping;
Expand All @@ -53,6 +49,7 @@
import org.hibernate.sql.model.ast.builder.TableUpdateBuilder;
import org.hibernate.sql.model.ast.builder.TableUpdateBuilderSkipped;
import org.hibernate.sql.model.ast.builder.TableUpdateBuilderStandard;
import org.hibernate.sql.model.internal.MutationOperationGroupFactory;
import org.hibernate.sql.model.jdbc.JdbcMutationOperation;
import org.hibernate.tuple.entity.EntityMetamodel;

Expand Down Expand Up @@ -874,38 +871,6 @@ private static void optimisticLock(
} );
}

private void breakDownKeyJdbcValues(
Object id,
Object rowId,
SharedSessionContractImplementor session,
JdbcValueBindings jdbcValueBindings,
EntityTableMapping tableMapping) {
// if the mutation is against the identifier table and we need to use row-id...
if ( tableMapping.isIdentifierTable() && entityPersister().hasRowId() && rowId != null ) {
// todo (mutation) : make sure the SQL uses row-id in this case
jdbcValueBindings.bindValue(
rowId,
tableMapping.getTableName(),
entityPersister().getRowIdMapping().getRowIdName(),
ParameterUsage.RESTRICT
);
}
else {
tableMapping.getKeyMapping().breakDownKeyJdbcValues(
id,
(jdbcValue, columnMapping) -> {
jdbcValueBindings.bindValue(
jdbcValue,
tableMapping.getTableName(),
columnMapping.getColumnName(),
ParameterUsage.RESTRICT
);
},
session
);
}
}

private static void decomposeAttributeMapping(
SharedSessionContractImplementor session,
JdbcValueBindings jdbcValueBindings,
Expand Down Expand Up @@ -1100,7 +1065,6 @@ private void applyTableUpdateDetails(
DirtinessChecker dirtinessChecker,
SharedSessionContractImplementor session) {
final EntityVersionMapping versionMapping = entityPersister().getVersionMapping();
final EntityRowIdMapping rowIdMapping = entityPersister().getRowIdMapping();
final AttributeMappingsList attributeMappings = entityPersister().getAttributeMappings();
final boolean[] versionability = entityPersister().getPropertyVersionability();
final OptimisticLockStyle optimisticLockStyle = entityPersister().optimisticLockStyle();
Expand Down Expand Up @@ -1155,7 +1119,7 @@ private void applyTableUpdateDetails(
updateGroupBuilder.forEachTableMutationBuilder( (builder) -> {
final EntityTableMapping tableMapping = (EntityTableMapping) builder.getMutatingTable().getTableMapping();
final TableUpdateBuilder<?> tableUpdateBuilder = (TableUpdateBuilder<?>) builder;
applyKeyRestriction( rowId, rowIdMapping, tableUpdateBuilder, tableMapping );
applyKeyRestriction( rowId, entityPersister(), tableUpdateBuilder, tableMapping );
applyPartitionKeyRestriction( tableUpdateBuilder );
} );
}
Expand All @@ -1177,19 +1141,6 @@ private void applyPartitionKeyRestriction(TableUpdateBuilder<?> tableUpdateBuild
}
}

private static void applyKeyRestriction(
Object rowId,
EntityRowIdMapping rowIdMapping,
TableUpdateBuilder<?> tableUpdateBuilder,
EntityTableMapping tableMapping) {
if ( rowIdMapping != null && rowId != null && tableMapping.isIdentifierTable() ) {
tableUpdateBuilder.addKeyRestrictionLeniently( rowIdMapping );
}
else {
tableUpdateBuilder.addKeyRestrictions( tableMapping.getKeyMapping() );
}
}

private static void applyAttributeLockingDetails(
Object[] oldValues,
SharedSessionContractImplementor session,
Expand Down Expand Up @@ -1344,7 +1295,7 @@ else if ( dirtyAttributeIndexes != null ) {
|| entityPersister().optimisticLockStyle() == DIRTY ) {
tablesNeedingDynamicUpdate.add( tableMapping );
}
else if ( rowId == null && entityPersister().getRowIdMapping() != null && tableMapping.isIdentifierTable() ) {
else if ( rowId == null && needsRowId( entityPersister(), tableMapping ) ) {
tablesNeedingDynamicUpdate.add( tableMapping );
}
}
Expand Down
Loading