Fix HQL update issues with composite id fk associations as fix some raw types issue

This commit is contained in:
Christian Beikov 2022-03-29 13:53:32 +02:00
parent 52fd9cfe17
commit 21a343fc60
15 changed files with 191 additions and 84 deletions

View File

@ -205,7 +205,7 @@ public class QuerySplitter {
}
@Override
public SqmAssignment visitAssignment(SqmAssignment assignment) {
public SqmAssignment<?> visitAssignment(SqmAssignment<?> assignment) {
throw new UnsupportedOperationException( "Not valid" );
}

View File

@ -546,8 +546,9 @@ public class SemanticQueryBuilder<R> extends HqlParserBaseVisitor<Object> implem
for ( ParseTree subCtx : setClauseCtx.children ) {
if ( subCtx instanceof HqlParser.AssignmentContext ) {
final HqlParser.AssignmentContext assignmentContext = (HqlParser.AssignmentContext) subCtx;
//noinspection unchecked
updateStatement.applyAssignment(
consumeDomainPath( (HqlParser.SimplePathContext) assignmentContext.getChild( 0 ) ),
(SqmPath<Object>) consumeDomainPath( (HqlParser.SimplePathContext) assignmentContext.getChild( 0 ) ),
(SqmExpression<?>) assignmentContext.getChild( 2 ).accept( this )
);
}

View File

@ -106,7 +106,7 @@ public interface SemanticQueryWalker<T> {
T visitSetClause(SqmSetClause setClause);
T visitAssignment(SqmAssignment assignment);
T visitAssignment(SqmAssignment<?> assignment);
T visitInsertSelectStatement(SqmInsertSelectStatement<?> statement);

View File

@ -84,6 +84,7 @@ import org.hibernate.query.sqm.tree.SqmTypedNode;
import org.hibernate.query.sqm.tree.delete.SqmDeleteStatement;
import org.hibernate.query.sqm.tree.domain.SqmPath;
import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter;
import org.hibernate.query.sqm.tree.expression.SqmExpression;
import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper;
import org.hibernate.query.sqm.tree.expression.SqmParameter;
import org.hibernate.query.sqm.tree.insert.SqmInsertSelectStatement;
@ -93,6 +94,7 @@ import org.hibernate.query.sqm.tree.insert.SqmValues;
import org.hibernate.query.sqm.tree.select.SqmQueryPart;
import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
import org.hibernate.query.sqm.tree.select.SqmSelection;
import org.hibernate.query.sqm.tree.update.SqmAssignment;
import org.hibernate.query.sqm.tree.update.SqmUpdateStatement;
import org.hibernate.sql.results.internal.TupleMetadata;
@ -302,8 +304,9 @@ public class QuerySqmImpl<R>
);
}
if ( sqmStatement instanceof SqmUpdateStatement<?> ) {
SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqmStatement;
final SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqmStatement;
verifyImmutableEntityUpdate( hql, updateStatement, getSessionFactory() );
verifyUpdateTypesMatch( hql, updateStatement );
}
else if ( sqmStatement instanceof SqmInsertStatement<?> ) {
verifyInsertTypesMatch( hql, (SqmInsertStatement<R>) sqmStatement );
@ -342,6 +345,31 @@ public class QuerySqmImpl<R>
}
}
private void verifyUpdateTypesMatch(String hqlString, SqmUpdateStatement<R> sqmStatement) {
final List<SqmAssignment<?>> assignments = sqmStatement.getSetClause().getAssignments();
for ( int i = 0; i < assignments.size(); i++ ) {
final SqmAssignment<?> assignment = assignments.get( i );
final SqmPath<?> targetPath = assignment.getTargetPath();
final SqmExpression<?> expression = assignment.getValue();
if ( targetPath.getNodeJavaType() == null || expression.getNodeJavaType() == null ) {
continue;
}
if ( targetPath.getNodeJavaType() != expression.getNodeJavaType()
&& !targetPath.getNodeJavaType().isWider( expression.getNodeJavaType() ) ) {
throw new SemanticException(
String.format(
"The assignment expression type [%s] did not match the assignment path type [%s] for the path [%s]",
expression.getNodeJavaType().getJavaType().getTypeName(),
targetPath.getNodeJavaType().getJavaType().getTypeName(),
targetPath.toHqlString()
),
hqlString,
null
);
}
}
}
private void verifyInsertTypesMatch(String hqlString, SqmInsertStatement<R> sqmStatement) {
final List<SqmPath<?>> insertionTargetPaths = sqmStatement.getInsertionTargetPaths();
if ( sqmStatement instanceof SqmInsertValuesStatement<?> ) {

View File

@ -383,7 +383,7 @@ public class SqmTreePrinter implements SemanticQueryWalker<Object> {
}
@Override
public Object visitAssignment(SqmAssignment assignment) {
public Object visitAssignment(SqmAssignment<?> assignment) {
processStanza(
"assignment",
() -> {

View File

@ -155,8 +155,8 @@ public class MultiTableSqmMutationConverter extends BaseSqmToSqlAstConverter<Sta
SqmParameterResolutionConsumer parameterResolutionConsumer) {
this.parameterResolutionConsumer = parameterResolutionConsumer;
for ( SqmAssignment assignment : setClause.getAssignments() ) {
assignmentConsumer.accept( visitAssignment( assignment ) );
for ( Assignment assignment : super.visitSetClause( setClause ) ) {
assignmentConsumer.accept( assignment );
}
}
@ -164,13 +164,6 @@ public class MultiTableSqmMutationConverter extends BaseSqmToSqlAstConverter<Sta
throw new UnsupportedOperationException();
}
@Override
public Assignment visitAssignment(SqmAssignment sqmAssignment) {
return new Assignment(
(Assignable) sqmAssignment.getTargetPath().accept( this ),
(Expression) sqmAssignment.getValue().accept( this )
);
}
/**
* Specialized hook to visit the assignments defined by the update SQM allow
* "listening" for each SQL assignment.
@ -239,10 +232,13 @@ public class MultiTableSqmMutationConverter extends BaseSqmToSqlAstConverter<Sta
}
@Override
protected Expression consumeSingleSqmParameter(SqmParameter<?> sqmParameter) {
protected Expression consumeSqmParameter(
SqmParameter<?> sqmParameter,
MappingModelExpressible<?> valueMapping,
BiConsumer<Integer, JdbcParameter> jdbcParameterConsumer) {
assert parameterResolutionConsumer != null;
final Expression expression = super.consumeSingleSqmParameter( sqmParameter );
final Expression expression = super.consumeSqmParameter( sqmParameter, valueMapping, jdbcParameterConsumer );
final List<List<JdbcParameter>> jdbcParameters = getJdbcParamsBySqmParam().get( sqmParameter );
final MappingModelExpressible<?> mappingType = getSqmParameterMappingModelExpressibleResolutions().get( sqmParameter );

View File

@ -144,7 +144,7 @@ public abstract class BaseSemanticQueryWalker implements SemanticQueryWalker<Obj
@Override
public Object visitSetClause(SqmSetClause setClause) {
if ( setClause != null ) {
for ( SqmAssignment assignment : setClause.getAssignments() ) {
for ( SqmAssignment<?> assignment : setClause.getAssignments() ) {
visitAssignment( assignment );
}
}
@ -152,7 +152,7 @@ public abstract class BaseSemanticQueryWalker implements SemanticQueryWalker<Obj
}
@Override
public Object visitAssignment(SqmAssignment assignment) {
public Object visitAssignment(SqmAssignment<?> assignment) {
assignment.getTargetPath().accept( this );
assignment.getValue().accept( this );
return assignment;

View File

@ -837,7 +837,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
public List<Assignment> visitSetClause(SqmSetClause setClause) {
final List<Assignment> assignments = new ArrayList<>( setClause.getAssignments().size() );
for ( SqmAssignment sqmAssignment : setClause.getAssignments() ) {
for ( SqmAssignment<?> sqmAssignment : setClause.getAssignments() ) {
final List<ColumnReference> targetColumnReferences = new ArrayList<>();
pushProcessingState(
@ -4546,12 +4546,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
return consumeSqmParameter( expression );
}
protected Expression consumeSqmParameter(
SqmParameter<?> sqmParameter,
BiConsumer<Integer, JdbcParameter> jdbcParameterConsumer) {
return consumeSqmParameter( sqmParameter, determineValueMapping( sqmParameter ), jdbcParameterConsumer );
}
protected Expression consumeSqmParameter(
SqmParameter<?> sqmParameter,
MappingModelExpressible<?> valueMapping,
@ -4650,37 +4644,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
}
protected Expression consumeSingleSqmParameter(SqmParameter<?> sqmParameter) {
final MappingModelExpressible<?> valueMapping = determineValueMapping( sqmParameter );
final List<JdbcParameter> jdbcParametersForSqm = new ArrayList<>();
resolveSqmParameter(
sqmParameter,
valueMapping,
jdbcParametersForSqm::add
);
this.jdbcParameters.addParameters( jdbcParametersForSqm );
this.jdbcParamsBySqmParam
.computeIfAbsent( sqmParameter, k -> new ArrayList<>( 1 ) )
.add( jdbcParametersForSqm );
final QueryParameterImplementor<?> queryParameter = domainParameterXref.getQueryParameter( sqmParameter );
final QueryParameterBinding binding = domainParameterBindings.getBinding( queryParameter );
if ( binding.setType( valueMapping ) ) {
replaceJdbcParametersType(
sqmParameter,
domainParameterXref.getSqmParameters( queryParameter ),
valueMapping
);
}
return new SqmParameterInterpretation(
sqmParameter,
queryParameter,
jdbcParametersForSqm,
valueMapping,
qp -> binding
);
return consumeSqmParameter( sqmParameter, determineValueMapping( sqmParameter ), (integer, jdbcParameter) -> {} );
}
@Override

View File

@ -13,33 +13,31 @@ import org.hibernate.query.sqm.tree.expression.SqmExpression;
/**
* @author Steve Ebersole
*/
public class SqmAssignment {
private final SqmPath targetPath;
private final SqmExpression value;
public class SqmAssignment<T> {
private final SqmPath<T> targetPath;
private final SqmExpression<? extends T> value;
public SqmAssignment(SqmPath targetPath, SqmExpression value) {
public SqmAssignment(SqmPath<T> targetPath, SqmExpression<? extends T> value) {
this.targetPath = targetPath;
this.value = value;
//noinspection unchecked
this.value.applyInferableType( targetPath.getNodeType() );
}
public SqmAssignment copy(SqmCopyContext context) {
return new SqmAssignment( targetPath.copy( context ), value.copy( context ) );
public SqmAssignment<T> copy(SqmCopyContext context) {
return new SqmAssignment<>( targetPath.copy( context ), value.copy( context ) );
}
/**
* The attribute/path to be updated
*/
public SqmPath getTargetPath() {
public SqmPath<T> getTargetPath() {
return targetPath;
}
/**
* The new value
*/
public SqmExpression getValue() {
public SqmExpression<? extends T> getValue() {
return value;
}
}

View File

@ -18,33 +18,33 @@ import org.hibernate.query.sqm.tree.expression.SqmExpression;
* @author Steve Ebersole
*/
public class SqmSetClause {
private final List<SqmAssignment> assignments;
private final List<SqmAssignment<?>> assignments;
public SqmSetClause() {
this.assignments = new ArrayList<>();
}
private SqmSetClause(List<SqmAssignment> assignments) {
private SqmSetClause(List<SqmAssignment<?>> assignments) {
this.assignments = assignments;
}
public SqmSetClause copy(SqmCopyContext context) {
final List<SqmAssignment> assignments = new ArrayList<>( this.assignments.size() );
for ( SqmAssignment assignment : this.assignments ) {
final List<SqmAssignment<?>> assignments = new ArrayList<>( this.assignments.size() );
for ( SqmAssignment<?> assignment : this.assignments ) {
assignments.add( assignment.copy( context ) );
}
return new SqmSetClause( assignments );
}
public List<SqmAssignment> getAssignments() {
public List<SqmAssignment<?>> getAssignments() {
return Collections.unmodifiableList( assignments );
}
public void addAssignment(SqmAssignment assignment) {
public void addAssignment(SqmAssignment<?> assignment) {
assignments.add( assignment );
}
public void addAssignment(SqmPath targetPath, SqmExpression value) {
addAssignment( new SqmAssignment( targetPath, value ) );
public <Y> void addAssignment(SqmPath<Y> targetPath, SqmExpression<? extends Y> value) {
addAssignment( new SqmAssignment<>( targetPath, value ) );
}
}

View File

@ -23,7 +23,6 @@ import org.hibernate.query.sqm.tree.domain.SqmPath;
import org.hibernate.query.sqm.tree.expression.SqmExpression;
import org.hibernate.query.sqm.tree.expression.SqmParameter;
import org.hibernate.query.sqm.tree.from.SqmRoot;
import org.hibernate.query.sqm.tree.predicate.SqmWhereClause;
import jakarta.persistence.criteria.Expression;
import jakarta.persistence.criteria.Path;
@ -78,7 +77,7 @@ public class SqmUpdateStatement<T>
}
final SqmUpdateStatement<T> statement = context.registerCopy(
this,
new SqmUpdateStatement<T>(
new SqmUpdateStatement<>(
nodeBuilder(),
getQuerySource(),
copyParameters( context ),
@ -111,25 +110,29 @@ public class SqmUpdateStatement<T>
@Override
public <Y> SqmUpdateStatement<T> set(SingularAttribute<? super T, Y> attribute, Expression<? extends Y> value) {
applyAssignment( getTarget().get( attribute ), (SqmExpression<?>) value );
//noinspection unchecked
applyAssignment( getTarget().get( attribute ), (SqmExpression<? extends Y>) value );
return this;
}
@Override
public <Y, X extends Y> SqmUpdateStatement<T> set(Path<Y> attribute, X value) {
applyAssignment( (SqmPath<?>) attribute, nodeBuilder().value( value ) );
applyAssignment( (SqmPath<Y>) attribute, nodeBuilder().value( value ) );
return this;
}
@Override
public <Y> SqmUpdateStatement<T> set(Path<Y> attribute, Expression<? extends Y> value) {
applyAssignment( (SqmPath<?>) attribute, (SqmExpression<?>) value );
//noinspection unchecked
applyAssignment( (SqmPath<Y>) attribute, (SqmExpression<? extends Y>) value );
return this;
}
@Override
public SqmUpdateStatement<T> set(String attributeName, Object value) {
applyAssignment( getTarget().get( attributeName ), nodeBuilder().value( value ) );
//noinspection unchecked
final SqmPath<Object> sqmPath = (SqmPath<Object>) getTarget().get( attributeName );
applyAssignment( sqmPath, nodeBuilder().value( value ) );
return this;
}
@ -167,11 +170,11 @@ public class SqmUpdateStatement<T>
return walker.visitUpdateStatement( this );
}
public void applyAssignment(SqmPath targetPath, SqmExpression value) {
public <Y> void applyAssignment(SqmPath<Y> targetPath, SqmExpression<? extends Y> value) {
if ( setClause == null ) {
setClause = new SqmSetClause();
}
setClause.addAssignment( new SqmAssignment( targetPath, value ) );
setClause.addAssignment( new SqmAssignment<>( targetPath, value ) );
}
@Override
@ -183,7 +186,7 @@ public class SqmUpdateStatement<T>
sb.append( getTarget().getEntityName() );
sb.append( ' ' ).append( getTarget().resolveAlias() );
sb.append( " set " );
final List<SqmAssignment> assignments = setClause.getAssignments();
final List<SqmAssignment<?>> assignments = setClause.getAssignments();
appendAssignment( assignments.get( 0 ), sb );
for ( int i = 1; i < assignments.size(); i++ ) {
sb.append( ", " );
@ -193,7 +196,7 @@ public class SqmUpdateStatement<T>
super.appendHqlString( sb );
}
private static void appendAssignment(SqmAssignment sqmAssignment, StringBuilder sb) {
private static void appendAssignment(SqmAssignment<?> sqmAssignment, StringBuilder sb) {
sqmAssignment.getTargetPath().appendHqlString( sb );
sb.append( " = " );
sqmAssignment.getValue().appendHqlString( sb );

View File

@ -13,6 +13,7 @@ import jakarta.persistence.EmbeddedId;
import jakarta.persistence.Entity;
import jakarta.persistence.Inheritance;
import jakarta.persistence.InheritanceType;
import jakarta.persistence.ManyToOne;
/**
* Entity having a many to one in its pk
@ -25,4 +26,7 @@ public class Child {
@EmbeddedId
@AttributeOverride(name = "nthChild", column = @Column(name = "nth"))
public ChildPk id;
@ManyToOne
public Parent parent1;
}

View File

@ -103,6 +103,15 @@ public class CompositeIdTest {
);
}
@Test
public void testUpdateCompositeIdFkAssociation(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
session.createQuery( "update Child c set c.parent1 = null" ).executeUpdate();
}
);
}
/**
* This feature is not supported by the EJB3

View File

@ -0,0 +1,61 @@
package org.hibernate.orm.test.jpa;
import org.hibernate.query.SemanticException;
import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
import org.hibernate.testing.orm.junit.Jpa;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import jakarta.persistence.Query;
import static org.assertj.core.api.Assertions.assertThat;
@Jpa(annotatedClasses = {
EntityWithCompositeIdFkAssociation.class,
EntityWithCompositeId.class,
CompositeId.class
})
public class CompositeIdFkUpdateTest {
@Test
public void testUpdateAssociationSetNull(EntityManagerFactoryScope scope) {
scope.inTransaction(
entityManager -> {
Query q = entityManager.createQuery(
"update EntityWithCompositeIdFkAssociation e set e.association = null" );
q.executeUpdate();
}
);
}
@Test
public void testUpdateAssociationSetRubbish(EntityManagerFactoryScope scope) {
scope.inTransaction(
entityManager -> {
try {
entityManager.createQuery(
"update EntityWithCompositeIdFkAssociation e set e.association = 1" );
Assertions.fail( "Expected query type validation to fail due to illegal assignment" );
}
catch (IllegalArgumentException ex) {
assertThat( ex.getCause() ).isInstanceOf( SemanticException.class );
assertThat( ex.getCause() ).hasMessageContaining( "did not match" );
}
}
);
}
@Test
public void testUpdateAssociationSetAssociationPart(EntityManagerFactoryScope scope) {
scope.inTransaction(
entityManager -> {
Query q = entityManager.createQuery(
"update EntityWithCompositeIdFkAssociation e set e.association.id.id1 = 1" );
q.executeUpdate();
}
);
}
}

View File

@ -0,0 +1,43 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.orm.test.jpa;
import java.io.Serializable;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
/**
*
*/
@Entity
@Table(name = "entity_composite_fk")
public class EntityWithCompositeIdFkAssociation implements Serializable {
@Id
private int id;
@ManyToOne
private EntityWithCompositeId association;
public int getId() {
return id;
}
public void setId(int id) {
this.id = id;
}
public EntityWithCompositeId getAssociation() {
return association;
}
public void setAssociation(EntityWithCompositeId association) {
this.association = association;
}
}