HHH-5126 : "in" expression and column-valued-input-parameter

hibernate-core/src/main/antlr/hql.g

@@ -612,10 +612,14 @@ atom
 primaryExpression
 	:   identPrimary ( options {greedy=true;} : DOT^ "class" )?
 	|   constant
-	|   COLON^ identifier
+	|   parameter
 	// TODO: Add parens to the tree so the user can control the operator evaluation order.
 	|   OPEN! (expressionOrVector | subQuery) CLOSE!
-	|   PARAM^ (NUM_INT)?
+	;
+
+parameter
+	: COLON^ identifier
+	| PARAM^ (NUM_INT)?
 	;
 
 // This parses normal expression and a list of expressions separated by commas.  If a comma is encountered
@@ -676,6 +680,7 @@ compoundExpr
 	: collectionExpr
 	| path
 	| (OPEN! ( (expression (COMMA! expression)*) | subQuery ) CLOSE!)
+	| parameter
 	;
 
 subQuery

hibernate-core/src/main/java/org/hibernate/impl/AbstractQueryImpl.java

@@ -61,7 +61,6 @@ import org.hibernate.proxy.HibernateProxyHelper;
 import org.hibernate.transform.ResultTransformer;
 import org.hibernate.type.SerializableType;
 import org.hibernate.type.Type;
-import org.hibernate.type.TypeFactory;
 import org.hibernate.util.ArrayHelper;
 import org.hibernate.util.MarkerObject;
 import org.hibernate.util.ReflectHelper;
@@ -751,8 +750,35 @@ public abstract class AbstractQueryImpl implements Query {
 	private String expandParameterList(String query, String name, TypedValue typedList, Map namedParamsCopy) {
 		Collection vals = (Collection) typedList.getValue();
 		Type type = typedList.getType();
-		if ( vals.size() == 1 ) {
+
-			// short-circuit for performance...
+		boolean isJpaPositionalParam = parameterMetadata.getNamedParameterDescriptor( name ).isJpaStyle();
+		String paramPrefix = isJpaPositionalParam ? "?" : ParserHelper.HQL_VARIABLE_PREFIX;
+		String placeholder =
+				new StringBuffer( paramPrefix.length() + name.length() )
+						.append( paramPrefix ).append(  name )
+						.toString();
+
+		if ( query == null ) {
+			return query;
+		}
+		int loc = query.indexOf( placeholder );
+
+		if ( loc < 0 ) {
+			return query;
+		}
+
+		String beforePlaceholder = query.substring( 0, loc );
+		String afterPlaceholder =  query.substring( loc + placeholder.length() );
+
+		// check if placeholder is already immediately enclosed in parentheses
+		// (ignoring whitespace)
+		boolean isEnclosedInParens =
+				StringHelper.getLastNonWhitespaceCharacter( beforePlaceholder ) == '(' &&
+				StringHelper.getFirstNonWhitespaceCharacter( afterPlaceholder ) == ')';
+
+		if ( vals.size() == 1  && isEnclosedInParens ) {
+			// short-circuit for performance when only 1 value and the
+			// placeholder is already enclosed in parentheses...
 			namedParamsCopy.put( name, new TypedValue( type, vals.iterator().next(), session.getEntityMode() ) );
 			return query;
 		}
@@ -760,7 +786,6 @@ public abstract class AbstractQueryImpl implements Query {
 		StringBuffer list = new StringBuffer( 16 );
 		Iterator iter = vals.iterator();
 		int i = 0;
-		boolean isJpaPositionalParam = parameterMetadata.getNamedParameterDescriptor( name ).isJpaStyle();
 		while ( iter.hasNext() ) {
 			String alias = ( isJpaPositionalParam ? 'x' + name : name ) + i++ + '_';
 			namedParamsCopy.put( alias, new TypedValue( type, iter.next(), session.getEntityMode() ) );
@@ -769,8 +794,14 @@ public abstract class AbstractQueryImpl implements Query {
 				list.append( ", " );
 			}
 		}
-		String paramPrefix = isJpaPositionalParam ? "?" : ParserHelper.HQL_VARIABLE_PREFIX;
+		return StringHelper.replace(
-		return StringHelper.replace( query, paramPrefix + name, list.toString(), true );
+				beforePlaceholder,
+				afterPlaceholder,
+				placeholder.toString(),
+				list.toString(),
+				true,
+				true
+		);
 	}
 
 	public Query setParameterList(String name, Collection vals) throws HibernateException {

hibernate-core/src/main/java/org/hibernate/util/StringHelper.java

@@ -28,6 +28,7 @@ import java.util.Iterator;
 import java.util.StringTokenizer;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.regex.Pattern;
 
 import org.hibernate.dialect.Dialect;
 
@@ -108,6 +109,14 @@ public final class StringHelper {
 	}
 
 	public static String replace(String template, String placeholder, String replacement, boolean wholeWords) {
+		return replace( template, placeholder, replacement, wholeWords, false );
+	}
+
+	public static String replace(String template,
+								 String placeholder,
+								 String replacement,
+								 boolean wholeWords,
+								 boolean encloseInParensIfNecessary) {
 		if ( template == null ) {
 			return template;
 		}
@@ -116,20 +125,72 @@ public final class StringHelper {
 			return template;
 		}
 		else {
-			final boolean actuallyReplace = !wholeWords ||
+			String beforePlaceholder = template.substring( 0, loc );
-					loc + placeholder.length() == template.length() ||
+			String afterPlaceholder = template.substring( loc + placeholder.length() );
-					!Character.isJavaIdentifierPart( template.charAt( loc + placeholder.length() ) );
+			return replace( beforePlaceholder, afterPlaceholder, placeholder, replacement, wholeWords, encloseInParensIfNecessary );
-			String actualReplacement = actuallyReplace ? replacement : placeholder;
-			return new StringBuffer( template.substring( 0, loc ) )
-					.append( actualReplacement )
-					.append( replace( template.substring( loc + placeholder.length() ),
-							placeholder,
-							replacement,
-							wholeWords ) ).toString();
 		}
 	}
 
 
+	public static String replace(String beforePlaceholder,
+								 String afterPlaceholder,
+								 String placeholder,
+								 String replacement,
+								 boolean wholeWords,
+								 boolean encloseInParensIfNecessary) {
+		final boolean actuallyReplace =
+				! wholeWords ||
+				afterPlaceholder.length() == 0 ||
+				! Character.isJavaIdentifierPart( afterPlaceholder.charAt( 0 ) );
+		boolean encloseInParens =
+				actuallyReplace &&
+				encloseInParensIfNecessary &&
+				! ( getLastNonWhitespaceCharacter( beforePlaceholder ) == '(' ) &&
+				! ( getFirstNonWhitespaceCharacter( afterPlaceholder ) == ')' );		
+		StringBuilder buf = new StringBuilder( beforePlaceholder );
+		if ( encloseInParens ) {
+			buf.append( '(' );
+		}
+		buf.append( actuallyReplace ? replacement : placeholder );
+		if ( encloseInParens ) {
+			buf.append( ')' );
+		}
+		buf.append(
+				replace(
+						afterPlaceholder,
+						placeholder,
+						replacement,
+						wholeWords,
+						encloseInParensIfNecessary
+				)
+		);
+		return buf.toString();
+	}
+
+	public static char getLastNonWhitespaceCharacter(String str) {
+		if ( str != null && str.length() > 0 ) {
+			for ( int i = str.length() - 1 ; i >= 0 ; i-- ) {
+				char ch = str.charAt( i );
+				if ( ! Character.isWhitespace( ch ) ) {
+					return ch;
+				}
+			}
+		}
+		return '\0';
+	}
+
+	public static char getFirstNonWhitespaceCharacter(String str) {
+		if ( str != null && str.length() > 0 ) {
+			for ( int i = 0 ; i < str.length() ; i++ ) {
+				char ch = str.charAt( i );
+				if ( ! Character.isWhitespace( ch ) ) {
+					return ch;
+				}
+			}
+		}
+		return '\0';
+	}
+
 	public static String replaceOnce(String template, String placeholder, String replacement) {
 		if ( template == null ) {
 			return template; // returnign null!

hibernate-core/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java

@@ -282,7 +282,7 @@ public class CompositeIdTest extends TestCase {
         ids.add( new SomeEntityId(1,12) );
         ids.add( new SomeEntityId(10,23) );
         ids.add( new SomeEntityId(10,22) );
-        Query query=s.createQuery( "from SomeEntity e where e.id in (:idList)" );
+        Query query=s.createQuery( "from SomeEntity e where e.id in :idList" );
         query.setParameterList( "idList", ids );
         List list=query.list();
         assertEquals( 3, list.size() );

hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java

@@ -529,6 +529,11 @@ public class ASTParserLoadingTest extends FunctionalTestCase {
 		list.add( new Id("123456789", order.getId().getOrderNumber(), "1234") );
 		query.setParameterList( "idList", list );
 		assertEquals( 2, query.list().size() );
+
+		query = s.createQuery( "from LineItem l where l.id in :idList" );
+		query.setParameterList( "idList", list );
+		assertEquals( 2, query.list().size() );
+
 		s.getTransaction().rollback();
 		s.close();
 		
@@ -695,6 +700,11 @@ public class ASTParserLoadingTest extends FunctionalTestCase {
 		s.createQuery( "from Human where name.last in (?1)" )
 				.setParameterList( "1", params )
 				.list();
+
+		s.createQuery( "from Human where name.last in ?1" )
+				.setParameterList( "1", params )
+				.list();
+
 		s.getTransaction().commit();
 		s.close();
 	}

hibernate-core/src/test/java/org/hibernate/test/hql/HQLTest.java

@@ -107,6 +107,7 @@ public class HQLTest extends QueryTranslatorTestCase {
      */
     public void testRowValueConstructorSyntaxInInListFailureExpected() {
         assertTranslation( "from LineItem l where l.id in (:idList)" );
+		assertTranslation( "from LineItem l where l.id in :idList" );
     }
 
     public void testRowValueConstructorSyntaxInInList() {
@@ -114,10 +115,14 @@ public class HQLTest extends QueryTranslatorTestCase {
     		return;
 		QueryTranslatorImpl translator = createNewQueryTranslator("from LineItem l where l.id in (?)");
 		assertInExist("'in' should be translated to 'and'", false, translator);
+		translator = createNewQueryTranslator("from LineItem l where l.id in ?");
+		assertInExist("'in' should be translated to 'and'", false, translator);
 		translator = createNewQueryTranslator("from LineItem l where l.id in (('a1',1,'b1'),('a2',2,'b2'))");
 		assertInExist("'in' should be translated to 'and'", false, translator);
 		translator = createNewQueryTranslator("from Animal a where a.id in (?)");
 		assertInExist("only translate tuple with 'in' syntax", true, translator);
+		translator = createNewQueryTranslator("from Animal a where a.id in ?");
+		assertInExist("only translate tuple with 'in' syntax", true, translator);
 		translator = createNewQueryTranslator("from LineItem l where l.id in (select a1 from Animal a1 left join a1.offspring o where a1.id = 1)");
 		assertInExist("do not translate subqueries", true, translator);
 

hibernate-core/src/test/java/org/hibernate/test/legacy/SQLFunctionsTest.java

@@ -169,10 +169,18 @@ public class SQLFunctionsTest extends LegacyTestCase {
 		q.setProperties(single);
 		assertTrue( q.list().get(0)==simple );
 
+		q = s.createQuery("from Simple s where s.name in :several");
+		q.setProperties(single);
+		assertTrue( q.list().get(0)==simple );
 
 		q = s.createQuery("from Simple s where s.name in (:stuff)");
 		q.setProperties(single);
 		assertTrue( q.list().get(0)==simple );
+
+		q = s.createQuery("from Simple s where s.name in :stuff");
+		q.setProperties(single);
+		assertTrue( q.list().get(0)==simple );
+
 		s.delete(simple);
 		t.commit();
 		s.close();

hibernate-core/src/test/java/org/hibernate/test/legacy/SQLLoaderTest.java

@@ -112,9 +112,21 @@ public class SQLLoaderTest extends LegacyTestCase {
 			query = session.createSQLQuery("select {category.*} from category {category} where {category}.name in (:names)", "category", Category.class);
 			String[] str = new String[] { "WannaBeFound", "NotThere" };
 			query.setParameterList("names", str);
-			
 			query.uniqueResult();
-			
+
+			query = session.createSQLQuery("select {category.*} from category {category} where {category}.name in :names", "category", Category.class);
+			query.setParameterList("names", str);
+			query.uniqueResult();
+
+			query = session.createSQLQuery("select {category.*} from category {category} where {category}.name in (:names)", "category", Category.class);
+			str = new String[] { "WannaBeFound" };
+			query.setParameterList("names", str);
+			query.uniqueResult();
+
+			query = session.createSQLQuery("select {category.*} from category {category} where {category}.name in :names", "category", Category.class);
+			query.setParameterList("names", str);			
+			query.uniqueResult();
+
 			session.connection().commit();
 			session.close();
 			

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/query/QueryTest.java

@@ -4,6 +4,9 @@ package org.hibernate.ejb.test.query;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
+
 import javax.persistence.EntityManager;
 import javax.persistence.Query;
 import javax.persistence.TemporalType;
@@ -14,7 +17,6 @@ import org.hibernate.ejb.test.Item;
 import org.hibernate.ejb.test.Wallet;
 import org.hibernate.ejb.test.Distributor;
 
-
 /**
  * @author Emmanuel Bernard
  */
@@ -64,6 +66,54 @@ public class QueryTest extends TestCase {
 		assertTrue( em.contains( item ) );
 		em.getTransaction().commit();
 
+		em.getTransaction().begin();
+		Query q = em.createQuery( "select item from Item item where item.name in :names" );
+		//test hint in value and string
+		q.setHint( "org.hibernate.fetchSize", 10 );
+		q.setHint( "org.hibernate.fetchSize", "10" );
+		List params = new ArrayList();
+		params.add( item.getName() );
+		q.setParameter( "names", params );
+		List result = q.getResultList();
+		assertNotNull( result );
+		assertEquals( 1, result.size() );
+
+		q = em.createQuery( "select item from Item item where item.name in :names" );
+		//test hint in value and string
+		q.setHint( "org.hibernate.fetchSize", 10 );
+		q.setHint( "org.hibernate.fetchSize", "10" );
+		params.add( item2.getName() );
+		q.setParameter( "names", params );
+		result = q.getResultList();
+		assertNotNull( result );
+		assertEquals( 2, result.size() );
+
+		q = em.createQuery( "select item from Item item where item.name in ?1" );
+		params = new ArrayList();
+		params.add( item.getName() );
+		params.add( item2.getName() );
+		q.setParameter( "1", params );
+		result = q.getResultList();
+		assertNotNull( result );
+		assertEquals( 2, result.size() );
+		em.remove( result.get( 0 ) );
+		em.remove( result.get( 1 ) );
+		em.getTransaction().commit();
+
+		em.close();
+	}
+
+	public void testParameterListInExistingParens() throws Exception {
+		final Item item = new Item( "Mouse", "Micro$oft mouse" );
+		final Item item2 = new Item( "Computer", "Dell computer" );
+
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+		em.persist( item );
+		em.persist( item2 );
+		assertTrue( em.contains( item ) );
+		em.getTransaction().commit();
+
 		em.getTransaction().begin();
 		Query q = em.createQuery( "select item from Item item where item.name in (:names)" );
 		//test hint in value and string
@@ -77,6 +127,18 @@ public class QueryTest extends TestCase {
 		assertNotNull( result );
 		assertEquals( 2, result.size() );
 
+		q = em.createQuery( "select item from Item item where item.name in ( \n :names \n)\n" );
+		//test hint in value and string
+		q.setHint( "org.hibernate.fetchSize", 10 );
+		q.setHint( "org.hibernate.fetchSize", "10" );
+		params = new ArrayList();
+		params.add( item.getName() );
+		params.add( item2.getName() );
+		q.setParameter( "names", params );
+		result = q.getResultList();
+		assertNotNull( result );
+		assertEquals( 2, result.size() );
+
 		q = em.createQuery( "select item from Item item where item.name in ( ?1 )" );
 		params = new ArrayList();
 		params.add( item.getName() );
@@ -201,7 +263,7 @@ public class QueryTest extends TestCase {
 		em.persist( w );
 		em.getTransaction().commit();
 		em.getTransaction().begin();
-		Query query = em.createQuery( "select w from " + Wallet.class.getName() + " w where w.brand in (?1)" );
+		Query query = em.createQuery( "select w from " + Wallet.class.getName() + " w where w.brand in ?1" );
 		List brands = new ArrayList();
 		brands.add( "Lacoste" );
 		query.setParameter( 1, brands );