From f15e4aaafc9e7825ea61e5b5d2dd940fa0a41eb9 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 1 Jul 2024 10:13:05 +0200 Subject: [PATCH 1/2] HHH-18321 Add test for issue --- .../orm/test/jpa/criteria/CoalesceTest.java | 115 ++++++++++++------ 1 file changed, 75 insertions(+), 40 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/CoalesceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/CoalesceTest.java index 1f0c10c1065e..74bacd882c78 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/CoalesceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/CoalesceTest.java @@ -6,15 +6,25 @@ */ package org.hibernate.orm.test.jpa.criteria; +import java.math.BigDecimal; +import java.util.List; + import org.hibernate.testing.TestForIssue; import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.Jira; import org.hibernate.testing.orm.junit.Jpa; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; import jakarta.persistence.Entity; import jakarta.persistence.Id; +import jakarta.persistence.Tuple; import jakarta.persistence.TypedQuery; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; @@ -22,13 +32,18 @@ import jakarta.persistence.criteria.ParameterExpression; import jakarta.persistence.criteria.Root; +import static org.assertj.core.api.Assertions.assertThat; + /** * @author Will Dazy */ -@Jpa(annotatedClasses = CoalesceTest.HHH15291Entity.class) +@Jpa( annotatedClasses = { + CoalesceTest.HHH15291Entity.class, + CoalesceTest.ComponentEntity.class, + CoalesceTest.ComponentA.class, +} ) @TestForIssue( jiraKey = "HHH-15291") public class CoalesceTest { - @Test public void hhh15291JPQL1Test(EntityManagerFactoryScope scope) { scope.inEntityManager( @@ -97,9 +112,46 @@ public void hhh15291Criteria2Test(EntityManagerFactoryScope scope) { ); } + @Test + @Jira( "https://hibernate.atlassian.net/browse/HHH-18321" ) + public void testCoalesceInBinaryArithmetic(EntityManagerFactoryScope scope) { + scope.inTransaction( entityManager -> { + final CriteriaBuilder cb = entityManager.getCriteriaBuilder(); + final CriteriaQuery cquery = cb.createTupleQuery(); + final Root root = cquery.from( ComponentEntity.class ); + + cquery.multiselect( + root.get( "id" ), + cb.diff( + cb.coalesce( root.get( "componentA" ).get( "income" ), BigDecimal.ZERO ), + cb.coalesce( root.get( "componentA" ).get( "expense" ), BigDecimal.ZERO ) + ) + ); + + final List resultList = entityManager.createQuery( cquery ).getResultList(); + assertThat( resultList ).hasSize( 2 ); + for ( Tuple result : resultList ) { + final Long id = result.get( 0, Long.class ); + assertThat( result.get( 1, BigDecimal.class ).intValue() ).isEqualTo( id == 1L ? 0 : 1 ); + } + } ); + } + + @BeforeAll + public void setUp(EntityManagerFactoryScope scope) { + scope.inTransaction( entityManager -> { + entityManager.persist( new ComponentEntity( 1L, new ComponentA( BigDecimal.ONE, BigDecimal.ONE ) ) ); + entityManager.persist( new ComponentEntity( 2L, new ComponentA( BigDecimal.ONE, null ) ) ); + } ); + } + + @AfterAll + public void tearDown(EntityManagerFactoryScope scope) { + scope.inTransaction( entityManager -> entityManager.createQuery( "delete from ComponentEntity" ).executeUpdate() ); + } + @Entity(name = "HHH15291Entity") public static class HHH15291Entity { - @Id @Column(name = "KEY_CHAR") private String KeyString; @@ -118,53 +170,36 @@ public static class HHH15291Entity { @Column(name = "ITEM_INTEGER1") private Integer itemInteger1; + } - public String getKeyString() { - return KeyString; - } - - public void setKeyString(String keyString) { - KeyString = keyString; - } - - public String getItemString1() { - return itemString1; - } - - public void setItemString1(String itemString1) { - this.itemString1 = itemString1; - } - - public String getItemString2() { - return itemString2; - } - - public void setItemString2(String itemString2) { - this.itemString2 = itemString2; - } + @Entity( name = "ComponentEntity" ) + static class ComponentEntity { + @Id + private Long id; - public String getItemString3() { - return itemString3; - } + @Embedded + private ComponentA componentA; - public void setItemString3(String itemString3) { - this.itemString3 = itemString3; + public ComponentEntity() { } - public String getItemString4() { - return itemString4; + public ComponentEntity(Long id, ComponentA componentA) { + this.id = id; + this.componentA = componentA; } + } - public void setItemString4(String itemString4) { - this.itemString4 = itemString4; - } + @Embeddable + static class ComponentA { + private BigDecimal income; + private BigDecimal expense; - public Integer getItemInteger1() { - return itemInteger1; + public ComponentA() { } - public void setItemInteger1(Integer itemInteger1) { - this.itemInteger1 = itemInteger1; + public ComponentA(BigDecimal income, BigDecimal expense) { + this.income = income; + this.expense = expense; } } } From fec90926de7baa4ed202c5a73a72f9a8fa58f6c3 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 1 Jul 2024 10:13:13 +0200 Subject: [PATCH 2/2] HHH-18321 Only infer temporal type from other operand when needed Also, defer temporal value mapping computation after visitation --- .../sqm/sql/BaseSqmToSqlAstConverter.java | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index d1d1cd416c91..5ebb3ba4765b 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -6562,19 +6562,9 @@ public Object visitBinaryArithmeticExpression(SqmBinaryArithmetic expression) SqmExpression leftOperand = expression.getLeftHandOperand(); SqmExpression rightOperand = expression.getRightHandOperand(); - // Need to infer the operand types here first to decide how to transform the expression - final FromClauseIndex fromClauseIndex = fromClauseIndexStack.getCurrent(); - inferrableTypeAccessStack.push( () -> determineValueMapping( rightOperand, fromClauseIndex ) ); - final MappingModelExpressible leftOperandType = determineValueMapping( leftOperand ); - inferrableTypeAccessStack.pop(); - inferrableTypeAccessStack.push( () -> determineValueMapping( leftOperand, fromClauseIndex ) ); - final MappingModelExpressible rightOperandType = determineValueMapping( rightOperand ); - inferrableTypeAccessStack.pop(); - final boolean durationToRight = isDuration( rightOperand.getNodeType() ); - final TypeConfiguration typeConfiguration = getCreationContext().getMappingMetamodel().getTypeConfiguration(); - final TemporalType temporalTypeToLeft = typeConfiguration.getSqlTemporalType( leftOperandType ); - final TemporalType temporalTypeToRight = typeConfiguration.getSqlTemporalType( rightOperandType ); + final TemporalType temporalTypeToLeft = inferTemporalType( leftOperand, rightOperand ); + final TemporalType temporalTypeToRight = inferTemporalType( rightOperand, leftOperand ); final boolean temporalTypeSomewhereToLeft = adjustedTimestamp != null || temporalTypeToLeft != null; if ( temporalTypeToLeft != null && durationToRight ) { @@ -6592,6 +6582,7 @@ else if ( temporalTypeToLeft != null && temporalTypeToRight != null ) { } else { // Infer one operand type through the other + final FromClauseIndex fromClauseIndex = fromClauseIndexStack.getCurrent(); inferrableTypeAccessStack.push( () -> determineValueMapping( rightOperand, fromClauseIndex ) ); final Expression lhs = toSqlExpression( leftOperand.accept( this ) ); inferrableTypeAccessStack.pop(); @@ -6620,6 +6611,20 @@ else if ( temporalTypeToLeft != null && temporalTypeToRight != null ) { } } + private TemporalType inferTemporalType(SqmExpression expression, SqmExpression inferrableOperand) { + final SqmExpressible nodeType = expression.getNodeType(); + if ( nodeType != null ) { + return getTypeConfiguration().getSqlTemporalType( nodeType ); + } + else { + // Need to infer the type here first to determine the correct temporal type + final FromClauseIndex fromClauseIndex = fromClauseIndexStack.getCurrent(); + inferrableTypeAccessStack.push( () -> determineValueMapping( inferrableOperand, fromClauseIndex ) ); + final MappingModelExpressible mappingType = determineValueMapping( expression ); + return TypeConfiguration.getSqlTemporalType( mappingType ); + } + } + private BasicValuedMapping getExpressionType(SqmExpression expression) { final SqmExpressible nodeType = expression.getNodeType(); if ( nodeType != null ) {