From 1e07c4f85fb2d06109b3b70b58f7a36727600cc0 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Tue, 18 Jan 2022 17:13:29 +0100 Subject: [PATCH] Fix expression type for arithmetic operations --- .../sqm/internal/SqmCriteriaNodeBuilder.java | 20 ++-- .../sqm/sql/BaseSqmToSqlAstConverter.java | 21 ++-- .../hibernate/type/spi/TypeConfiguration.java | 16 +-- .../orm/test/jpa/compliance/ProdTest.java | 103 ++++++++++++++++++ 4 files changed, 126 insertions(+), 34 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/ProdTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java index 749e167928..52a1cc7e30 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java @@ -715,7 +715,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return createSqmArithmeticNode( BinaryArithmeticOperator.ADD, (SqmExpression) x, - value( y, (SqmExpression) x ) + value( y ) ); } @@ -724,7 +724,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, public SqmExpression sum(N x, Expression y) { return createSqmArithmeticNode( BinaryArithmeticOperator.ADD, - value( x, (SqmExpression) y ), + value( x ), (SqmExpression) y ); } @@ -743,7 +743,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return createSqmArithmeticNode( BinaryArithmeticOperator.MULTIPLY, (SqmExpression) x, - value( y, (SqmExpression) x ) + value( y ) ); } @@ -751,7 +751,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, public SqmExpression prod(N x, Expression y) { return createSqmArithmeticNode( BinaryArithmeticOperator.MULTIPLY, - value( x, (SqmExpression) y ), + value( x ), (SqmExpression) y ); } @@ -771,7 +771,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return createSqmArithmeticNode( BinaryArithmeticOperator.SUBTRACT, (SqmExpression) x, - value( y, (SqmExpression) x ) + value( y ) ); } @@ -779,7 +779,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, public SqmExpression diff(N x, Expression y) { return createSqmArithmeticNode( BinaryArithmeticOperator.SUBTRACT, - value( x, (SqmExpression) y ), + value( x ), (SqmExpression) y ); } @@ -798,7 +798,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return createSqmArithmeticNode( BinaryArithmeticOperator.QUOT, (SqmExpression) x, - value( y, (SqmExpression) x ) + value( y ) ); } @@ -807,7 +807,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, public SqmExpression quot(Number x, Expression y) { return createSqmArithmeticNode( BinaryArithmeticOperator.QUOT, - value( x, (SqmExpression) y ), + value( x ), (SqmExpression) y ); } @@ -826,7 +826,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return createSqmArithmeticNode( BinaryArithmeticOperator.MODULO, (SqmExpression) x, - value( y, (SqmExpression) x ) + value( y ) ); } @@ -834,7 +834,7 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, public SqmExpression mod(Integer x, Expression y) { return createSqmArithmeticNode( BinaryArithmeticOperator.MODULO, - value( x, (SqmExpression) y ), + value( x ), (SqmExpression) y ); } 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 4208bafc3c..de95d33705 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 @@ -4685,17 +4685,16 @@ public abstract class BaseSqmToSqlAstConverter extends Base } private BasicValuedMapping getExpressionType(SqmBinaryArithmetic expression) { - final SqmExpressable sqmExpressable = QueryHelper.highestPrecedenceType( - expression.getLeftHandOperand().getNodeType(), - expression.getRightHandOperand().getNodeType() - ); - if ( sqmExpressable instanceof BasicValuedMapping ) { - return (BasicValuedMapping) sqmExpressable; - } - else if ( sqmExpressable != null ) { - return getTypeConfiguration().getBasicTypeForJavaType( - sqmExpressable.getExpressableJavaTypeDescriptor().getJavaTypeClass() - ); + final SqmExpressable nodeType = expression.getNodeType(); + if ( nodeType != null ) { + if ( nodeType instanceof BasicValuedMapping ) { + return (BasicValuedMapping) nodeType; + } + else { + return getTypeConfiguration().getBasicTypeForJavaType( + nodeType.getExpressableJavaTypeDescriptor().getJavaTypeClass() + ); + } } return JavaObjectType.INSTANCE; } diff --git a/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java b/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java index 3549742662..70a52b7e27 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java +++ b/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java @@ -511,19 +511,18 @@ public class TypeConfiguration implements SessionFactoryObserver, Serializable { SqmExpressable firstType, SqmExpressable secondType, BinaryArithmeticOperator operator) { - return resolveArithmeticType( firstType, secondType, operator == DIVIDE ); + return resolveArithmeticType( firstType, secondType ); } /** * Determine the result type of an arithmetic operation as defined by the - * rules in section 6.5.7.1. + * rules in section 6.5.8.1. * * @see QueryHelper#highestPrecedenceType2 */ public SqmExpressable resolveArithmeticType( SqmExpressable firstType, - SqmExpressable secondType, - boolean isDivision) { + SqmExpressable secondType) { if ( getSqlTemporalType( firstType ) != null ) { if ( secondType==null || getSqlTemporalType( secondType ) != null ) { @@ -548,15 +547,6 @@ public class TypeConfiguration implements SessionFactoryObserver, Serializable { return getBasicTypeRegistry().getRegisteredType( Duration.class ); } - if ( isDivision ) { - // covered under the note in 6.5.7.1 discussing the unportable - // "semantics of the SQL division operation".. - return getBasicTypeRegistry().getRegisteredType( Number.class.getName() ); - } - - - // non-division - if ( matchesJavaType( firstType, Double.class ) ) { return firstType; } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/ProdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/ProdTest.java new file mode 100644 index 0000000000..cc65347e93 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/ProdTest.java @@ -0,0 +1,103 @@ +/* + * 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.compliance; + +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.Jpa; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Root; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +@Jpa( + annotatedClasses = ProdTest.Person.class +) +public class ProdTest { + @BeforeEach + public void setUp(EntityManagerFactoryScope scope) { + scope.inTransaction( + entityManager -> { + entityManager.persist( new Person( 1, "Luigi ", 42 ) ); + } + ); + } + + @AfterEach + public void tearDown(EntityManagerFactoryScope scope) { + scope.inTransaction( + entityManager -> + entityManager.createQuery( "delete from Person" ).executeUpdate() + ); + } + + @Test + public void testCriteriaMod(EntityManagerFactoryScope scope) { + scope.inEntityManager( + entityManager -> { + final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder(); + final CriteriaQuery query = criteriaBuilder.createQuery( Number.class ); + final Root person = query.from( Person.class ); + query.select( criteriaBuilder.prod( person.get( "age" ), 1F ) ); + + final Number id = entityManager.createQuery( query ).getSingleResult(); + + assertInstanceOf( Float.class, id ); + assertEquals( 42F, id.floatValue() ); + } + ); + } + + @Test + public void testQueryMod(EntityManagerFactoryScope scope) { + scope.inEntityManager( + entityManager -> { + final Object id = entityManager.createQuery( "select p.age * 1F from Person p" ) + .getSingleResult(); + assertInstanceOf( Float.class, id ); + assertEquals( 42F, id ); + } + ); + } + + @Entity(name = "Person") + @Table(name = "PERSON_TABLE") + public static class Person { + + @Id + private Integer id; + + private String name; + + private Integer age; + + Person() { + } + + public Person(Integer id, String name, Integer age) { + this.id = id; + this.name = name; + this.age = age; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + } +}