Author: stliu Date: 2009-12-21 12:05:58 -0500 (Mon, 21 Dec 2009) New Revision: 18305 Added: core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java Modified: core/trunk/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java Log: HHH-2166 Long 'in' lists in queries results in a Java stack overflow exception Modified: core/trunk/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java 2009-12-21 17:02:33 UTC (rev 18304) +++ core/trunk/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java 2009-12-21 17:05:58 UTC (rev 18305) @@ -25,44 +25,132 @@ package org.hibernate.hql.ast.util; import antlr.collections.AST; +import java.util.Map; +import java.util.HashMap; /** * A visitor for traversing an AST tree. - * + * * @author Steve Ebersole + * @author Philip R. "Pib" Burns. + * */ + public class NodeTraverser { public static interface VisitationStrategy { - public void visit(AST node); + public void visit( AST node ); } private final VisitationStrategy strategy; - public NodeTraverser(VisitationStrategy strategy) { + public NodeTraverser( VisitationStrategy strategy ) { this.strategy = strategy; } /** * Traverse the AST tree depth first. - * <p/> - * Note that the AST passed in is not visited itself. Visitation starts - * with its children. - * + * * @param ast + * Root node of subtree to traverse. + * + * <p> + * Note that the AST passed in is not visited itself. Visitation + * starts with its children. + * </p> + * <p> + * The current code for traverseDepthFirst uses iteration to walk + * the tree. This corrects stack overflow problems for constructs + * such as "x in (:x)" where ":x" specifies a large number of + * items. + * </p> */ - public void traverseDepthFirst(AST ast) { + + public void traverseDepthFirst( AST ast ) { + // Root AST node cannot be null or + // traversal of its subtree is impossible. if ( ast == null ) { throw new IllegalArgumentException( "node to traverse cannot be null!" ); } - visitDepthFirst( ast.getFirstChild() ); - } + // Map to hold parents of each + // AST node. Unfortunately the AST + // interface does not provide a method + // for finding the parent of a node, so + // we use the Map to save them. - private void visitDepthFirst(AST ast) { - if ( ast == null ) { - return; + Map parentNodes = new HashMap(); + + // Start tree traversal with first child + // of the specified root AST node. + + AST currentNode = ast.getFirstChild(); + + // Remember parent of first child. + + parentNodes.put(currentNode, ast); + + // Iterate through nodes, simulating + // recursive tree traversal, and add them + // to queue in proper order for later + // linear traversal. This "flattens" the + // into a linear list of nodes which can + // be visited non-recursively. + + while ( currentNode != null ) { + // Visit the current node. + + strategy.visit( currentNode ); + + // Move down to current node's first child + // if it exists. + + AST childNode = currentNode.getFirstChild(); + + // If the child is not null, make it + // the current node. + + if ( childNode != null ) { + // Remember parent of the child. + + parentNodes.put( childNode, currentNode ); + + // Make child the current node. + + currentNode = childNode; + + continue; + } + + while ( currentNode != null ) { + // Move to next sibling if any. + + AST siblingNode = currentNode.getNextSibling(); + + if (siblingNode != null) { + // Get current node's parent. + // This is also the parent of the + // sibling node. + + AST parentNode = (AST) parentNodes.get(currentNode); + + // Remember parent of sibling. + + parentNodes.put(siblingNode, parentNode); + + // Make sibling the current node. + + currentNode = siblingNode; + + break; + } + // Move up to parent if no sibling. + // If parent is root node, we're done. + + currentNode = (AST) parentNodes.get(currentNode); + + if (currentNode.equals(ast)) { + currentNode = null; + } + } } - strategy.visit( ast ); - visitDepthFirst( ast.getFirstChild() ); - visitDepthFirst( ast.getNextSibling() ); } } Added: core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml (rev 0) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml 2009-12-21 17:05:58 UTC (rev 18305) @@ -0,0 +1,17 @@ +<?xml version="1.0"?> +<!DOCTYPE hibernate-mapping SYSTEM "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd" > +<hibernate-mapping + package="org.hibernate.test.hql"> + + + + <class name="StateProvince"> + <id name="id"> + <generator class="native"/> + </id> + <property name="name"/> + <property name="isoCode"/> + </class> + + +</hibernate-mapping> \ No newline at end of file Added: core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java (rev 0) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java 2009-12-21 17:05:58 UTC (rev 18305) @@ -0,0 +1,111 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2008, Red Hat Middleware LLC 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 Middleware LLC. + * + * 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.test.criteria; + +import java.util.ArrayList; +import java.util.List; + +import org.hibernate.Criteria; +import org.hibernate.Query; +import org.hibernate.Session; +import org.hibernate.Transaction; +import org.hibernate.criterion.Restrictions; +import org.hibernate.junit.functional.FunctionalTestCase; +import org.hibernate.test.hql.StateProvince; + +/** + * + * HHH-2166 Long "in" lists in queries results in a Java stack overflow exception. + * + * @author Strong Liu + */ +public class LongInElementsTest extends FunctionalTestCase { + + private static final int ELEMENTS_SIZE = 4000; + + public LongInElementsTest( String string ) { + super(string); + } + + public String[] getMappings() { + return new String[] { "criteria/Animal.hbm.xml" }; + } + + //HHH-1123 + public void testLongInElementsByHQL(){ + Session session = openSession(); + Transaction t = session.beginTransaction(); + + StateProvince beijing = new StateProvince(); + beijing.setIsoCode("100089"); + beijing.setName("beijing"); + session.persist(beijing); + session.flush(); + session.clear(); + + Query query = session.createQuery("from org.hibernate.test.hql.StateProvince sp where sp.id in ( :idList )"); + query.setParameterList( "idList" , createLotsOfElements() ); + List list = query.list(); + session.flush(); + session.clear(); + assertEquals( 1, list.size() ); + session.delete(beijing); + t.commit(); + session.close(); + + } + + //HHH-1123 + public void te2stLongInElementsByCriteria(){ + Session session = openSession(); + Transaction t = session.beginTransaction(); + + StateProvince beijing = new StateProvince(); + beijing.setIsoCode("100089"); + beijing.setName("beijing"); + session.persist(beijing); + session.flush(); + session.clear(); + + Criteria criteria = session.createCriteria(StateProvince.class); + criteria.add(Restrictions.in("id", createLotsOfElements())); + List list = criteria.list(); + session.flush(); + session.clear(); + assertEquals( 1, list.size() ); + session.delete(beijing); + t.commit(); + session.close(); + + } + + private List createLotsOfElements(){ + List list = new ArrayList(); + for ( int i = 0; i < ELEMENTS_SIZE; i++ ){ + list.add(Long.valueOf(i)); + } + return list; + } +}