From ac2dcd9c5529f8170c401a029d6ba7a9c292e0cd Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Fri, 21 Jan 2011 12:43:32 -0800 Subject: [PATCH] HHH-5126 : "in" expression and column-valued-input-parameter --- core/src/main/antlr/hql.g | 9 ++- .../org/hibernate/impl/AbstractQueryImpl.java | 43 ++++++++-- .../java/org/hibernate/util/StringHelper.java | 81 ++++++++++++++++--- .../hibernate/ejb/test/query/QueryTest.java | 66 ++++++++++++++- .../test/annotations/cid/CompositeIdTest.java | 2 +- .../test/hql/ASTParserLoadingTest.java | 10 +++ .../java/org/hibernate/test/hql/HQLTest.java | 5 ++ .../test/legacy/SQLFunctionsTest.java | 8 ++ .../hibernate/test/legacy/SQLLoaderTest.java | 16 +++- 9 files changed, 217 insertions(+), 23 deletions(-) diff --git a/core/src/main/antlr/hql.g b/core/src/main/antlr/hql.g index 855715c9a419..5fe8917b39e5 100644 --- a/core/src/main/antlr/hql.g +++ b/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 diff --git a/core/src/main/java/org/hibernate/impl/AbstractQueryImpl.java b/core/src/main/java/org/hibernate/impl/AbstractQueryImpl.java index c46d208e9eed..11ef5bf743ec 100644 --- a/core/src/main/java/org/hibernate/impl/AbstractQueryImpl.java +++ b/core/src/main/java/org/hibernate/impl/AbstractQueryImpl.java @@ -61,7 +61,6 @@ 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 @@ protected String expandParameterLists(Map namedParamsCopy) { 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 @@ private String expandParameterList(String query, String name, TypedValue typedLi 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 @@ private String expandParameterList(String query, String name, TypedValue typedLi list.append( ", " ); } } - String paramPrefix = isJpaPositionalParam ? "?" : ParserHelper.HQL_VARIABLE_PREFIX; - return StringHelper.replace( query, paramPrefix + name, list.toString(), true ); + return StringHelper.replace( + beforePlaceholder, + afterPlaceholder, + placeholder.toString(), + list.toString(), + true, + true + ); } public Query setParameterList(String name, Collection vals) throws HibernateException { diff --git a/core/src/main/java/org/hibernate/util/StringHelper.java b/core/src/main/java/org/hibernate/util/StringHelper.java index 898ac1e3b426..13193ffe5f8b 100644 --- a/core/src/main/java/org/hibernate/util/StringHelper.java +++ b/core/src/main/java/org/hibernate/util/StringHelper.java @@ -28,6 +28,7 @@ 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 static String[] replace(String templates[], String placeholder, String re } 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,19 +125,71 @@ public static String replace(String template, String placeholder, String replace return template; } else { - final boolean actuallyReplace = !wholeWords || - loc + placeholder.length() == template.length() || - !Character.isJavaIdentifierPart( template.charAt( loc + placeholder.length() ) ); - 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(); + String beforePlaceholder = template.substring( 0, loc ); + String afterPlaceholder = template.substring( loc + placeholder.length() ); + return replace( beforePlaceholder, afterPlaceholder, placeholder, replacement, wholeWords, encloseInParensIfNecessary ); + } + } + + + 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 ) { diff --git a/entitymanager/src/test/java/org/hibernate/ejb/test/query/QueryTest.java b/entitymanager/src/test/java/org/hibernate/ejb/test/query/QueryTest.java index b64cc8c4d1c7..73dcea9c07a8 100644 --- a/entitymanager/src/test/java/org/hibernate/ejb/test/query/QueryTest.java +++ b/entitymanager/src/test/java/org/hibernate/ejb/test/query/QueryTest.java @@ -4,6 +4,9 @@ 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.Wallet; import org.hibernate.ejb.test.Distributor; - /** * @author Emmanuel Bernard */ @@ -64,6 +66,54 @@ public void testParameterList() throws Exception { 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 void testParameterList() throws Exception { 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 void testExplicitPositionalParameter() throws Exception { 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 ); diff --git a/testsuite/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java b/testsuite/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java index 425954c0eaf1..467b9ec72bf4 100644 --- a/testsuite/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java +++ b/testsuite/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java @@ -282,7 +282,7 @@ public void testQueryInAndCompositeWithHQL() { 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() ); diff --git a/testsuite/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java b/testsuite/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java index 41c2629bfec2..8f77c393a4f5 100644 --- a/testsuite/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java +++ b/testsuite/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java @@ -529,6 +529,11 @@ public void testRowValueConstructorSyntaxInInList() { 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 void testJPAPositionalParameterList() { 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(); } diff --git a/testsuite/src/test/java/org/hibernate/test/hql/HQLTest.java b/testsuite/src/test/java/org/hibernate/test/hql/HQLTest.java index 7a77e69f3373..3b89b90b77eb 100644 --- a/testsuite/src/test/java/org/hibernate/test/hql/HQLTest.java +++ b/testsuite/src/test/java/org/hibernate/test/hql/HQLTest.java @@ -107,6 +107,7 @@ public void testInvalidCollectionDereferencesFail() { */ 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 void testRowValueConstructorSyntaxInInList() { 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); diff --git a/testsuite/src/test/java/org/hibernate/test/legacy/SQLFunctionsTest.java b/testsuite/src/test/java/org/hibernate/test/legacy/SQLFunctionsTest.java index de43dd9635cd..00f2799e2de7 100644 --- a/testsuite/src/test/java/org/hibernate/test/legacy/SQLFunctionsTest.java +++ b/testsuite/src/test/java/org/hibernate/test/legacy/SQLFunctionsTest.java @@ -169,10 +169,18 @@ public void testSetProperties() throws Exception { 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(); diff --git a/testsuite/src/test/java/org/hibernate/test/legacy/SQLLoaderTest.java b/testsuite/src/test/java/org/hibernate/test/legacy/SQLLoaderTest.java index e435a06f8f74..d96434f27e57 100644 --- a/testsuite/src/test/java/org/hibernate/test/legacy/SQLLoaderTest.java +++ b/testsuite/src/test/java/org/hibernate/test/legacy/SQLLoaderTest.java @@ -112,9 +112,21 @@ public void testFindBySQLProperties() throws HibernateException, SQLException { 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();