HHH-16573 NPE with embeddable element collection with updateable = false

This commit is contained in:
Andrea Boriero 2023-05-10 12:52:13 +02:00 committed by Andrea Boriero
parent b6721961dd
commit aa93bac008
3 changed files with 107 additions and 43 deletions

View File

@ -64,6 +64,18 @@ public interface ValuedModelPart extends ModelPart, ValueMapping, SelectableMapp
);
}
default void forEachNonFormula(SelectableConsumer consumer) {
ModelPart.super.forEachSelectable(
(selectionIndex, selectableMapping) -> {
if ( selectableMapping.isFormula() ) {
return;
}
consumer.accept( selectionIndex, selectableMapping );
}
);
}
default void forEachUpdatable(SelectableConsumer consumer) {
ModelPart.super.forEachSelectable(
(selectionIndex, selectableMapping) -> {

View File

@ -464,8 +464,32 @@ public class BasicCollectionPersister extends AbstractCollectionPersister {
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// SET
attribute.getElementDescriptor().forEachUpdatable( updateBuilder );
if ( hasIndex() ) {
/*
Given :
class MyEntity {
@ElementCollection(fetch = FetchType.LAZY)
@OrderColumn
private List<MyEmbeddable> myEmbeddables;
}
@Embeddable
public static class MyEmbeddable {
@Column(updatable = false)
private String embeddedProperty;
}
we cannot understand if the update is due to a change in the value embeddedProperty or because a
new element has been added to the list in an existing position (update) shifting the old value (insert).
For this reason we cannot take into consideration the `@Column(updatable = false)`
*/
attribute.getElementDescriptor().forEachNonFormula( updateBuilder );
}
else {
attribute.getElementDescriptor().forEachUpdatable( updateBuilder );
}
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// WHERE
@ -501,16 +525,28 @@ public class BasicCollectionPersister extends AbstractCollectionPersister {
0,
jdbcValueBindings,
null,
(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) {
return;
}
bindings.bindValue(
jdbcValue,
jdbcValueMapping,
ParameterUsage.SET
);
},
hasIndex() ?
(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
if ( jdbcValueMapping.isFormula() ) {
return;
}
bindings.bindValue(
jdbcValue,
jdbcValueMapping,
ParameterUsage.SET
);
}
:
(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) {
return;
}
bindings.bindValue(
jdbcValue,
jdbcValueMapping,
ParameterUsage.SET
);
},
session
);
}

View File

@ -19,7 +19,10 @@ import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.metamodel.mapping.PluralAttributeMapping;
import org.hibernate.sql.model.MutationType;
import org.hibernate.sql.model.internal.AbstractMutationOperationGroup;
import org.hibernate.sql.model.internal.MutationOperationGroupNone;
import org.hibernate.sql.model.internal.MutationOperationGroupSingle;
import org.hibernate.sql.model.jdbc.JdbcMutationOperation;
/**
* UpdateRowsCoordinator implementation for cases with a separate collection table
@ -31,7 +34,7 @@ import org.hibernate.sql.model.internal.MutationOperationGroupSingle;
public class UpdateRowsCoordinatorStandard extends AbstractUpdateRowsCoordinator implements UpdateRowsCoordinator {
private final RowMutationOperations rowMutationOperations;
private MutationOperationGroupSingle operationGroup;
private AbstractMutationOperationGroup operationGroup;
public UpdateRowsCoordinatorStandard(
CollectionMutationTarget mutationTarget,
@ -43,7 +46,7 @@ public class UpdateRowsCoordinatorStandard extends AbstractUpdateRowsCoordinator
@Override
protected int doUpdate(Object key, PersistentCollection<?> collection, SharedSessionContractImplementor session) {
final MutationOperationGroupSingle operationGroup = getOperationGroup();
final AbstractMutationOperationGroup operationGroup = getOperationGroup();
final MutationExecutorService mutationExecutorService = session
.getFactory()
@ -115,42 +118,55 @@ public class UpdateRowsCoordinatorStandard extends AbstractUpdateRowsCoordinator
int entryPosition,
MutationExecutor mutationExecutor,
SharedSessionContractImplementor session) {
final PluralAttributeMapping attribute = getMutationTarget().getTargetPart();
if ( !collection.needsUpdating( entry, entryPosition, attribute ) ) {
if ( rowMutationOperations.getUpdateRowOperation() != null ) {
final PluralAttributeMapping attribute = getMutationTarget().getTargetPart();
if ( !collection.needsUpdating( entry, entryPosition, attribute ) ) {
return false;
}
rowMutationOperations.getUpdateRowValues().applyValues(
collection,
key,
entry,
entryPosition,
session,
mutationExecutor.getJdbcValueBindings()
);
rowMutationOperations.getUpdateRowRestrictions().applyRestrictions(
collection,
key,
entry,
entryPosition,
session,
mutationExecutor.getJdbcValueBindings()
);
mutationExecutor.execute( collection, null, null, null, session );
return true;
}
else {
return false;
}
rowMutationOperations.getUpdateRowValues().applyValues(
collection,
key,
entry,
entryPosition,
session,
mutationExecutor.getJdbcValueBindings()
);
rowMutationOperations.getUpdateRowRestrictions().applyRestrictions(
collection,
key,
entry,
entryPosition,
session,
mutationExecutor.getJdbcValueBindings()
);
mutationExecutor.execute( collection, null, null, null, session );
return true;
}
protected MutationOperationGroupSingle getOperationGroup() {
protected AbstractMutationOperationGroup getOperationGroup() {
if ( operationGroup == null ) {
operationGroup = new MutationOperationGroupSingle(
MutationType.UPDATE,
getMutationTarget(),
rowMutationOperations.getUpdateRowOperation()
);
final JdbcMutationOperation updateRowOperation = rowMutationOperations.getUpdateRowOperation();
if ( updateRowOperation == null ) {
operationGroup = new MutationOperationGroupNone(
MutationType.UPDATE,
getMutationTarget()
);
}
else {
operationGroup = new MutationOperationGroupSingle(
MutationType.UPDATE,
getMutationTarget(),
updateRowOperation
);
}
}
return operationGroup;
}