Better handling of implicit literal numeric expression typing

hibernate-entitymanager/src/main/java/org/hibernate/ejb/criteria/CriteriaBuilderImpl.java

@@ -77,6 +77,7 @@ import org.hibernate.ejb.criteria.predicate.ComparisonPredicate;
 import org.hibernate.ejb.criteria.predicate.ComparisonPredicate.ComparisonOperator;
 import org.hibernate.ejb.criteria.predicate.CompoundPredicate;
 import org.hibernate.ejb.criteria.predicate.ExistsPredicate;
+import org.hibernate.ejb.criteria.predicate.ImplicitNumericExpressionTypeDeterminer;
 import org.hibernate.ejb.criteria.predicate.InPredicate;
 import org.hibernate.ejb.criteria.predicate.IsEmptyPredicate;
 import org.hibernate.ejb.criteria.predicate.LikePredicate;
@@ -855,9 +856,7 @@ public class CriteriaBuilderImpl implements CriteriaBuilder, Serializable {
 
 	// arithmetic operations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	public <N extends Number> Expression<N> neg(Expression<N> expression) {
 		return new UnaryArithmeticOperation<N>(
 				this,
@@ -866,193 +865,228 @@ public class CriteriaBuilderImpl implements CriteriaBuilder, Serializable {
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> sum(Expression<? extends N> expression1, Expression<? extends N> expression2) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression1 );
+		if ( expression1 == null || expression2 == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, (Expression)expression2 );
+			throw new IllegalArgumentException( "arguments to sum() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression1.getJavaType(), expression2.getJavaType() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.ADD,
 				expression1,
 				expression2
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> prod(Expression<? extends N> expression1, Expression<? extends N> expression2) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression1 );
+		if ( expression1 == null || expression2 == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, (Expression)expression2 );
+			throw new IllegalArgumentException( "arguments to prod() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression1.getJavaType(), expression2.getJavaType() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.MULTIPLY,
 				expression1,
 				expression2
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> diff(Expression<? extends N> expression1, Expression<? extends N> expression2) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression1 );
+		if ( expression1 == null || expression2 == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, (Expression)expression2 );
+			throw new IllegalArgumentException( "arguments to diff() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression1.getJavaType(), expression2.getJavaType() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.SUBTRACT,
 				expression1,
 				expression2
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> sum(Expression<? extends N> expression, N n) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( expression == null || n == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to sum() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression.getJavaType(), n.getClass() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.ADD,
 				expression,
 				n
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> prod(Expression<? extends N> expression, N n) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( expression == null || n == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to prod() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression.getJavaType(), n.getClass() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.MULTIPLY,
 				expression,
 				n
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> diff(Expression<? extends N> expression, N n) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( expression == null || n == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to diff() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression.getJavaType(), n.getClass() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.SUBTRACT,
 				expression,
 				n
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> sum(N n, Expression<? extends N> expression) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( expression == null || n == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to sum() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( n.getClass(), expression.getJavaType() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.ADD,
 				n,
 				expression
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> prod(N n, Expression<? extends N> expression) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( n == null || expression == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to prod() cannot be null" );
-		return new BinaryArithmeticOperation<N>(
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( n.getClass(), expression.getJavaType() );
+
+		return (BinaryArithmeticOperation<N>) new BinaryArithmeticOperation(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.MULTIPLY,
 				n,
 				expression
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <N extends Number> Expression<N> diff(N n, Expression<? extends N> expression) {
-		Class<N> type = (Class<N>)BinaryArithmeticOperation.determineReturnType( (Class)Number.class, (Expression)expression );
+		if ( n == null || expression == null ) {
-		type = (Class<N>)BinaryArithmeticOperation.determineReturnType( type, n );
+			throw new IllegalArgumentException( "arguments to diff() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( n.getClass(), expression.getJavaType() );
+
 		return new BinaryArithmeticOperation<N>(
 				this,
-				type,
+				resultType,
 				BinaryArithmeticOperation.Operation.SUBTRACT,
 				n,
 				expression
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
+	@SuppressWarnings( {"unchecked"})
-	 */
 	public Expression<Number> quot(Expression<? extends Number> expression1, Expression<? extends Number> expression2) {
+		if ( expression1 == null || expression2 == null ) {
+			throw new IllegalArgumentException( "arguments to quot() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression1.getJavaType(), expression2.getJavaType(), true );
+
 		return new BinaryArithmeticOperation<Number>(
 				this,
-				Number.class,
+				resultType,
 				BinaryArithmeticOperation.Operation.DIVIDE,
 				expression1,
 				expression2
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
+	@SuppressWarnings( {"unchecked"})
-	 */
 	public Expression<Number> quot(Expression<? extends Number> expression, Number number) {
+		if ( expression == null || number == null ) {
+			throw new IllegalArgumentException( "arguments to quot() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( expression.getJavaType(), number.getClass(), true );
+
 		return new BinaryArithmeticOperation<Number>(
 				this,
-				Number.class,
+				resultType,
 				BinaryArithmeticOperation.Operation.DIVIDE,
 				expression,
 				number
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
+	@SuppressWarnings( {"unchecked"})
-	 */
 	public Expression<Number> quot(Number number, Expression<? extends Number> expression) {
+		if ( expression == null || number == null ) {
+			throw new IllegalArgumentException( "arguments to quot() cannot be null" );
+		}
+
+		final Class resultType = BinaryArithmeticOperation.determineResultType( number.getClass(), expression.getJavaType(), true );
+
 		return new BinaryArithmeticOperation<Number>(
 				this,
-				Number.class,
+				resultType,
 				BinaryArithmeticOperation.Operation.DIVIDE,
 				number,
 				expression
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	public Expression<Integer> mod(Expression<Integer> expression1, Expression<Integer> expression2) {
+		if ( expression1 == null || expression2 == null ) {
+			throw new IllegalArgumentException( "arguments to mod() cannot be null" );
+		}
+
 		return new BinaryArithmeticOperation<Integer>(
 				this,
 				Integer.class,
@@ -1062,10 +1096,12 @@ public class CriteriaBuilderImpl implements CriteriaBuilder, Serializable {
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	public Expression<Integer> mod(Expression<Integer> expression, Integer integer) {
+		if ( expression == null || integer == null ) {
+			throw new IllegalArgumentException( "arguments to mod() cannot be null" );
+		}
+
 		return new BinaryArithmeticOperation<Integer>(
 				this,
 				Integer.class,
@@ -1075,10 +1111,12 @@ public class CriteriaBuilderImpl implements CriteriaBuilder, Serializable {
 		);
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	public Expression<Integer> mod(Integer integer, Expression<Integer> expression) {
+		if ( integer == null || expression == null ) {
+			throw new IllegalArgumentException( "arguments to mod() cannot be null" );
+		}
+
 		return new BinaryArithmeticOperation<Integer>(
 				this,
 				Integer.class,

hibernate-entitymanager/src/main/java/org/hibernate/ejb/criteria/expression/BinaryArithmeticOperation.java

@@ -28,6 +28,7 @@ import org.hibernate.ejb.criteria.CriteriaBuilderImpl;
 import org.hibernate.ejb.criteria.CriteriaQueryCompiler;
 import org.hibernate.ejb.criteria.ParameterRegistry;
 import org.hibernate.ejb.criteria.Renderable;
+import org.hibernate.ejb.criteria.predicate.ImplicitNumericExpressionTypeDeterminer;
 
 /**
  * Models standard arithmetc operations with two operands.
@@ -89,6 +90,24 @@ public class BinaryArithmeticOperation<N extends Number>
 	private final Expression<? extends N> rhs;
 	private final Expression<? extends N> lhs;
 
+	public static Class<? extends Number> determineResultType(
+			Class<? extends Number> argument1Type,
+			Class<? extends Number> argument2Type
+	) {
+		return determineResultType( argument1Type, argument2Type, false );
+	}
+
+	public static Class<? extends Number> determineResultType(
+			Class<? extends Number> argument1Type,
+			Class<? extends Number> argument2Type,
+			boolean isQuotientOperation
+	) {
+		if ( isQuotientOperation ) {
+			return Number.class;
+		}
+		return ImplicitNumericExpressionTypeDeterminer.determineResultType( argument1Type, argument2Type );
+	}
+
 	/**
 	 * Helper for determining the appropriate operation return type based on one of the operands as an expression.
 	 *

hibernate-entitymanager/src/main/java/org/hibernate/ejb/criteria/predicate/ComparisonPredicate.java

@@ -75,6 +75,7 @@ public class ComparisonPredicate
 		}
 	}
 
+	@SuppressWarnings( {"unchecked"})
 	public <N extends Number> ComparisonPredicate(
 			CriteriaBuilderImpl criteriaBuilder,
 			ComparisonOperator comparisonOperator,
@@ -83,10 +84,14 @@ public class ComparisonPredicate
 		super( criteriaBuilder );
 		this.comparisonOperator = comparisonOperator;
 		this.leftHandSide = leftHandSide;
-		this.rightHandSide = new LiteralExpression<N>( 
+		Class type = leftHandSide.getJavaType();
-				criteriaBuilder,
+		if ( Number.class.equals( type ) ) {
-				ValueHandlerFactory.convert( rightHandSide, leftHandSide.getJavaType() )
+			this.rightHandSide = new LiteralExpression( criteriaBuilder, rightHandSide );
-		);
+		}
+		else {
+			N converted = (N) ValueHandlerFactory.convert( rightHandSide, type );
+			this.rightHandSide = new LiteralExpression<N>( criteriaBuilder, converted );
+		}
 	}
 
 	public ComparisonOperator getComparisonOperator() {

hibernate-entitymanager/src/main/java/org/hibernate/ejb/criteria/predicate/ImplicitNumericExpressionTypeDeterminer.java

@@ -0,0 +1,77 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2011, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.ejb.criteria.predicate;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+import org.hibernate.ejb.criteria.expression.BinaryArithmeticOperation.Operation;
+
+/**
+ * @author Steve Ebersole
+ */
+public class ImplicitNumericExpressionTypeDeterminer {
+	/**
+	 * Determine the appropriate runtime result type for a numeric expression according to
+	 * section "6.5.7.1 Result Types of Expressions" of the JPA spec.
+	 * <p/>
+	 * Note that it is expected that the caveats about quotient handling have already been handled.
+	 *
+	 * @param types The argument/expression types
+	 *
+	 * @return The appropriate numeric result type.
+	 */
+	public static Class<? extends Number> determineResultType(Class<? extends Number>... types) {
+		Class<? extends Number> result = Number.class;
+
+		for ( Class<? extends Number> type : types ) {
+			if ( Double.class.equals( type ) ) {
+				result = Double.class;
+			}
+			else if ( Float.class.equals( type ) ) {
+				result = Float.class;
+			}
+			else if ( BigDecimal.class.equals( type ) ) {
+				result = BigDecimal.class;
+			}
+			else if ( BigInteger.class.equals( type ) ) {
+				result = BigInteger.class;
+			}
+			else if ( Long.class.equals( type ) ) {
+				result = Long.class;
+			}
+			else if ( isIntegralType( type ) ) {
+				result = Integer.class;
+			}
+		}
+
+		return result;
+	}
+
+	private static boolean isIntegralType(Class<? extends Number> type) {
+		return Integer.class.equals( type ) ||
+				Short.class.equals( type );
+
+	}
+}

hibernate-entitymanager/src/test/java/org/hibernate/ejb/criteria/basic/PredicateTest.java

@@ -26,16 +26,21 @@ package org.hibernate.ejb.criteria.basic;
 import javax.persistence.EntityManager;
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.CriteriaQuery;
+import javax.persistence.criteria.Path;
 import javax.persistence.criteria.Predicate;
 import javax.persistence.criteria.Root;
 import java.util.List;
 
 import org.hibernate.ejb.metamodel.AbstractMetamodelSpecificTest;
+import org.hibernate.ejb.metamodel.Customer_;
 import org.hibernate.ejb.metamodel.Order;
+import org.hibernate.ejb.metamodel.Order_;
 
 import org.junit.Before;
 import org.junit.Test;
 
+import org.hibernate.testing.TestForIssue;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -220,5 +225,29 @@ public class PredicateTest extends AbstractMetamodelSpecificTest {
 		em.close();
 	}
 
+	@Test
+	@TestForIssue( jiraKey = "HHH-5803" )
+	public void testQuotientConversion() {
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+		CriteriaQuery<Order> orderCriteria = builder.createQuery( Order.class );
+		Root<Order> orderRoot = orderCriteria.from( Order.class );
+
+		Long longValue = 999999999L;
+		Path<Double> doublePath = orderRoot.get( Order_.totalPrice );
+		Path<Integer> integerPath = orderRoot.get( Order_.customer ).get( Customer_.age );
+
+		orderCriteria.select( orderRoot );
+		Predicate p = builder.ge(
+				builder.quot( integerPath, doublePath ),
+				longValue
+		);
+		orderCriteria.where( p );
+
+		List<Order> orders = em.createQuery( orderCriteria ).getResultList();
+		assertTrue( orders.size() == 0 );
+		em.getTransaction().commit();
+		em.close();
+	}
 
 }