HHH-15142 Reuse CriteriaQuery with explicit and implicit (=literal) params

hibernate-core/src/main/java/org/hibernate/query/criteria/internal/compile/CriteriaCompiler.java

@@ -11,7 +11,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import javax.persistence.Parameter;
 import javax.persistence.TypedQuery;
 import javax.persistence.criteria.ParameterExpression;
 
@@ -23,7 +22,6 @@
 import org.hibernate.internal.util.collections.Stack;
 import org.hibernate.internal.util.collections.StandardStack;
 import org.hibernate.query.criteria.LiteralHandlingMode;
-import org.hibernate.query.criteria.internal.expression.ParameterExpressionImpl;
 import org.hibernate.query.criteria.internal.expression.function.FunctionExpression;
 import org.hibernate.query.spi.QueryImplementor;
 import org.hibernate.sql.ast.Clause;
@@ -67,6 +65,7 @@ public QueryImplementor compile(CompilableCriteria criteria) {
 		RenderingContext renderingContext = new RenderingContext() {
 			private int aliasCount;
 			private int explicitParameterCount;
+			private int implicitParameterCount;
 
 			private final Stack<Clause> clauseStack = new StandardStack<>();
 			private final Stack<FunctionExpression> functionContextStack = new StandardStack<>();
@@ -79,6 +78,10 @@ public String generateParameterName() {
 				return "param" + explicitParameterCount++;
 			}
 
+			public String generateLiteralName() {
+				return "literal" + implicitParameterCount++;
+			}
+
 			@Override
 			public Stack<Clause> getClauseStack() {
 				return clauseStack;
@@ -109,9 +112,8 @@ else if ( criteriaQueryParameter.getPosition() != null ) {
 						);
 					}
 					else {
-						final String name = generateParameterName();
 						parameterInfo = new ExplicitParameterInfo(
-								name,
+								generateParameterName(),
 								null,
 								criteriaQueryParameter.getJavaType()
 						);
@@ -124,7 +126,7 @@ else if ( criteriaQueryParameter.getPosition() != null ) {
 			}
 
 			public String registerLiteralParameterBinding(final Object literal, final Class javaType) {
-				final String parameterName = generateParameterName();
+				final String parameterName = generateLiteralName();
 				final ImplicitParameterBinding binding = new ImplicitParameterBinding() {
 					public String getParameterName() {
 						return parameterName;
@@ -135,9 +137,6 @@ public Class getJavaType() {
 					}
 
 					public void bind(TypedQuery typedQuery) {
-						if ( literal instanceof Parameter ) {
-							return;
-						}
 						typedQuery.setParameter( parameterName, literal );
 					}
 				};

hibernate-core/src/test/java/org/hibernate/jpa/test/query/CriteriaUpdateWithParametersTest.java

@@ -1,10 +1,5 @@
 package org.hibernate.jpa.test.query;
 
-import org.hibernate.testing.TestForIssue;
-import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
-import org.hibernate.testing.orm.junit.Jpa;
-import org.junit.jupiter.api.Test;
-
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import javax.persistence.Query;
@@ -14,15 +9,25 @@
 import javax.persistence.criteria.Root;
 import javax.persistence.metamodel.EntityType;
 
-@Jpa(
-		annotatedClasses = CriteriaUpdateWithParametersTest.Person.class
-)
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.transaction.TransactionUtil;
+import org.junit.Test;
+
 @TestForIssue( jiraKey = "HHH-15113")
-public class CriteriaUpdateWithParametersTest {
+public class CriteriaUpdateWithParametersTest extends BaseEntityManagerFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {
+				Person.class
+		};
+	}
 
 	@Test
-	public void testCriteriaUpdate(EntityManagerFactoryScope scope) {
-		scope.inTransaction(
+	public void testCriteriaUpdate() {
+		TransactionUtil.doInJPA( this::entityManagerFactory,
 				entityManager -> {
 					final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
 					final CriteriaUpdate<Person> criteriaUpdate = criteriaBuilder.createCriteriaUpdate( Person.class );
@@ -50,8 +55,8 @@ public void testCriteriaUpdate(EntityManagerFactoryScope scope) {
 	}
 
 	@Test
-	public void testCriteriaUpdate2(EntityManagerFactoryScope scope) {
-		scope.inTransaction(
+	public void testCriteriaUpdate2() {
+		TransactionUtil.doInJPA( this::entityManagerFactory,
 				entityManager -> {
 					final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
 					final CriteriaUpdate<Person> criteriaUpdate = criteriaBuilder.createCriteriaUpdate( Person.class );

hibernate-core/src/test/java/org/hibernate/jpa/test/query/ReuseCriteriaWithMixedParametersTest.java

@@ -0,0 +1,156 @@
+/*
+ * 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.jpa.test.query;
+
+import java.time.Instant;
+import java.util.Date;
+import javax.persistence.Query;
+import javax.persistence.criteria.CriteriaBuilder;
+import javax.persistence.criteria.CriteriaQuery;
+import javax.persistence.criteria.ParameterExpression;
+import javax.persistence.criteria.Root;
+
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+import org.hibernate.jpa.test.Wallet;
+import org.hibernate.jpa.test.Wallet_;
+
+import org.hibernate.testing.TestForIssue;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
+
+@TestForIssue(jiraKey = "HHH-15113")
+public class ReuseCriteriaWithMixedParametersTest extends BaseEntityManagerFunctionalTestCase {
+
+	@Rule
+	public ExpectedException illegalString = ExpectedException.none();
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {
+				Wallet.class
+		};
+	}
+
+	@Test
+	public void cqReuse() {
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
+			final CriteriaQuery<Wallet> criteriaQuery = criteriaBuilder.createQuery( Wallet.class );
+			final Root<Wallet> root = criteriaQuery.from( Wallet.class );
+
+			final ParameterExpression<String> stringValueParameter = criteriaBuilder.parameter( String.class );
+
+			criteriaQuery.where(
+					criteriaBuilder.like(
+							root.get( Wallet_.model ),
+							stringValueParameter
+					),
+					criteriaBuilder.lessThan(
+							root.get( Wallet_.marketEntrance ),
+							criteriaBuilder.literal( Date.from( Instant.EPOCH ) )
+					)
+			);
+
+			Query query = entityManager.createQuery( criteriaQuery );
+			query.setParameter( stringValueParameter, "Z%" );
+
+			query.getResultList();
+
+			query = entityManager.createQuery( criteriaQuery );
+			//This throws due to ParameterExpressionImpl#name change
+			//causing marketEntrance literal to get the same name.
+			query.setParameter( stringValueParameter, "A%" );
+
+			query.getResultList();
+
+		} );
+	}
+
+	@Test
+	public void likeCqReuse() {
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
+			final CriteriaQuery<Wallet> criteriaQuery = criteriaBuilder.createQuery( Wallet.class );
+			final Root<Wallet> root = criteriaQuery.from( Wallet.class );
+
+			final ParameterExpression<String> stringValueParameter = criteriaBuilder.parameter( String.class );
+
+			criteriaQuery.where(
+					criteriaBuilder.like(
+							root.get( Wallet_.model ),
+							stringValueParameter,
+							'/'
+					)
+			);
+
+			Query query = entityManager.createQuery( criteriaQuery );
+			query.setParameter( stringValueParameter, "Z%" );
+
+			query.getResultList();
+
+			query = entityManager.createQuery( criteriaQuery );
+			//This throws due to ParameterExpressionImpl#name change
+			//causing like-escape literal to get the same name.
+			query.setParameter( stringValueParameter, "A%" );
+
+			query.getResultList();
+
+		} );
+	}
+
+	@Test
+	public void predicateReuse() {
+		//TODO: Unwanted behaviour.
+		illegalString.expect( IllegalArgumentException.class );
+		illegalString.expectMessage( "did not match expected type [java.lang.String (n/a)]" );
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
+			final CriteriaQuery<Wallet> criteriaQuery = criteriaBuilder.createQuery( Wallet.class );
+			final Root<Wallet> root = criteriaQuery.from( Wallet.class );
+
+			final ParameterExpression<String> stringValueParameter = criteriaBuilder.parameter( String.class );
+			final ParameterExpression<Date> dateValueParameter = criteriaBuilder.parameter( Date.class );
+
+			criteriaQuery.where(
+					criteriaBuilder.like(
+							root.get( Wallet_.model ),
+							stringValueParameter
+					)
+			);
+
+			Query query = entityManager.createQuery( criteriaQuery );
+			query.setParameter( stringValueParameter, "Z%" );
+
+			query.getResultList();
+			Assert.assertTrue( "No error", true );
+
+			criteriaQuery.where(
+					criteriaBuilder.like(
+							root.get( Wallet_.model ),
+							stringValueParameter
+					),
+					criteriaBuilder.lessThan(
+							root.get( Wallet_.marketEntrance ),
+							dateValueParameter
+					)
+			);
+
+			query = entityManager.createQuery( criteriaQuery );
+			query.setParameter( stringValueParameter, "A%" );
+			//This throws due to ParameterExpressionImpl#name change
+			//causing marketEntrance parameter to get the same name.
+			query.setParameter( dateValueParameter, Date.from( Instant.EPOCH ) );
+
+			query.getResultList();
+		} );
+	}
+
+}