From 95c69f3209426421bb230813f466d19cebe0a297 Mon Sep 17 00:00:00 2001 From: Strong Liu Date: Mon, 21 Dec 2009 17:05:58 +0000 Subject: [PATCH] HHH-2166 Long 'in' lists in queries results in a Java stack overflow exception git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18305 1b8cb986-b30d-0410-93ca-fae66ebed9b2 --- .../hibernate/hql/ast/util/NodeTraverser.java | 120 +++++++++++++++--- .../hibernate/test/criteria/Animal.hbm.xml | 17 +++ .../test/criteria/LongInElementsTest.java | 111 ++++++++++++++++ 3 files changed, 232 insertions(+), 16 deletions(-) create mode 100644 testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml create mode 100644 testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java diff --git a/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java b/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java index 2cef14944e..bb5468a2b9 100644 --- a/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java +++ b/core/src/main/java/org/hibernate/hql/ast/util/NodeTraverser.java @@ -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. - *

- * Note that the AST passed in is not visited itself. Visitation starts - * with its children. - * + * * @param ast + * Root node of subtree to traverse. + * + *

+ * Note that the AST passed in is not visited itself. Visitation + * starts with its children. + *

+ *

+ * 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. + *

*/ - 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() ); } } diff --git a/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml b/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml new file mode 100644 index 0000000000..b1e9e90d96 --- /dev/null +++ b/testsuite/src/test/java/org/hibernate/test/criteria/Animal.hbm.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java b/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java new file mode 100644 index 0000000000..7b6dd045ab --- /dev/null +++ b/testsuite/src/test/java/org/hibernate/test/criteria/LongInElementsTest.java @@ -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; + } +}