HHH-3750 : dialect first-result handling

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@15894 1b8cb986-b30d-0410-93ca-fae66ebed9b2

core/src/main/java/org/hibernate/dialect/Cache71Dialect.java

@@ -598,7 +598,7 @@ public class Cache71Dialect extends Dialect {
 
 	public String getLimitString(String sql, boolean hasOffset) {
 		if ( hasOffset ) {
-			throw new UnsupportedOperationException( "An offset may not be specified to <TOP n> in Cache SQL" );
+			throw new UnsupportedOperationException( "query result offset is not supported" );
 		}
 
 		// This does not support the Cache SQL 'DISTINCT BY (comma-list)' extensions,

core/src/main/java/org/hibernate/dialect/DB2390Dialect.java

@@ -48,15 +48,6 @@ public class DB2390Dialect extends DB2Dialect {
 		return false;
 	}
 
-	public String getLimitString(String sql, int offset, int limit) {
-		return new StringBuffer(sql.length() + 40)
-			.append(sql)
-			.append(" fetch first ")
-			.append(limit)
-			.append(" rows only ")
-			.toString();
-	}
-
 	public boolean useMaxForLimit() {
 		return true;
 	}
@@ -65,4 +56,16 @@ public class DB2390Dialect extends DB2Dialect {
 		return false;
 	}
 
+	public String getLimitString(String sql, int offset, int limit) {
+		if ( offset > 0 ) {
+			throw new UnsupportedOperationException( "query result offset is not supported" );
+		}
+		return new StringBuffer( sql.length() + 40 )
+				.append( sql )
+				.append( " fetch first " )
+				.append( limit )
+				.append( " rows only " )
+				.toString();
+	}
+
 }

core/src/main/java/org/hibernate/dialect/DB2400Dialect.java

@@ -25,10 +25,10 @@
 package org.hibernate.dialect;
 
 /**
-* An SQL dialect for DB2/400
-* @author Peter DeGregorio (pdegregorio)
-* This class provides support for DB2 Universal Database for iSeries,
+ * An SQL dialect for DB2/400.  This class provides support for DB2 Universal Database for iSeries,
  * also known as DB2/400.
+ *
+ * @author Peter DeGregorio (pdegregorio)
  */
 public class DB2400Dialect extends DB2Dialect {
 
@@ -48,15 +48,6 @@ public class DB2400Dialect extends DB2Dialect {
 		return false;
 	}
 
-	public String getLimitString(String sql, int offset, int limit) {
-		return new StringBuffer(sql.length() + 40)
-			.append(sql)
-			.append(" fetch first ")
-			.append(limit)
-			.append(" rows only ")
-			.toString();
-	}
-
 	public boolean useMaxForLimit() {
 		return true;
 	}
@@ -65,4 +56,16 @@ public class DB2400Dialect extends DB2Dialect {
 		return false;
 	}
 
+	public String getLimitString(String sql, int offset, int limit) {
+		if ( offset > 0 ) {
+			throw new UnsupportedOperationException( "query result offset is not supported" );
+		}
+		return new StringBuffer( sql.length() + 40 )
+				.append( sql )
+				.append( " fetch first " )
+				.append( limit )
+				.append( " rows only " )
+				.toString();
+	}
+
 }

core/src/main/java/org/hibernate/dialect/DB2Dialect.java

@@ -247,7 +247,6 @@ public class DB2Dialect extends Dialect {
 	}
 
 	public String getLimitString(String sql, boolean hasOffset) {
-
 		int startOfSelect = sql.toLowerCase().indexOf("select");
 
 		StringBuffer pagingSelect = new StringBuffer( sql.length()+100 )
@@ -277,6 +276,18 @@ public class DB2Dialect extends Dialect {
 		return pagingSelect.toString();
 	}
 
+	/**
+	 * DB2 does have a one-based offset, however this was actually already handled in the limiot string building
+	 * (the '?+1' bit).  To not mess up inheritors, I'll leave that part alone and not touch the offset here.
+	 *
+	 * @param zeroBasedFirstResult The user-supplied, zero-based offset
+	 *
+	 * @return zeroBasedFirstResult
+	 */
+	public int convertToFirstRowValue(int zeroBasedFirstResult) {
+		return zeroBasedFirstResult;
+	}
+
 	private static boolean hasDistinct(String sql) {
 		return sql.toLowerCase().indexOf("select distinct")>=0;
 	}

core/src/main/java/org/hibernate/dialect/Dialect.java

@@ -870,6 +870,25 @@ public abstract class Dialect {
 		throw new UnsupportedOperationException( "paged queries not supported" );
 	}
 
+	/**
+	 * Hibernate APIs explcitly state that setFirstResult() should be a zero-based offset. Here we allow the
+	 * Dialect a chance to convert that value based on what the underlying db or driver will expect.
+	 * <p/>
+	 * NOTE: what gets passed into {@link #getLimitString(String,int,int)} is the zero-based offset.  Dialects which
+	 * do not {@link #supportsVariableLimit} should take care to perform any needed {@link #convertToFirstRowValue}
+	 * calls prior to injecting the limit values into the SQL string.
+	 *
+	 * @param zeroBasedFirstResult The user-supplied, zero-based first row offset.
+	 *
+	 * @return The corresponding db/dialect specific offset.
+	 *
+	 * @see org.hibernate.Query#setFirstResult
+	 * @see org.hibernate.Criteria#setFirstResult
+	 */
+	public int convertToFirstRowValue(int zeroBasedFirstResult) {
+		return zeroBasedFirstResult;
+	}
+
 
 	// lock acquisition support ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

core/src/main/java/org/hibernate/dialect/FirebirdDialect.java

@@ -26,6 +26,7 @@ package org.hibernate.dialect;
 
 /**
  * An SQL dialect for Firebird.
+ *
  * @author Reha CENANI
  */
 public class FirebirdDialect extends InterbaseDialect {

core/src/main/java/org/hibernate/dialect/InformixDialect.java

@@ -110,11 +110,10 @@ public class InformixDialect extends Dialect {
 			String constraintName,
 			String[] foreignKey,
 			String referencedTable,
-			String[] primaryKey, boolean referencesPrimaryKey
-	) {
-		StringBuffer result = new StringBuffer(30);
-		
-		result.append(" add constraint ")
+			String[] primaryKey,
+			boolean referencesPrimaryKey) {
+		StringBuffer result = new StringBuffer( 30 )
+				.append( " add constraint " )
 				.append( " foreign key (" )
 				.append( StringHelper.join( ", ", foreignKey ) )
 				.append( ") references " )
@@ -172,7 +171,9 @@ public class InformixDialect extends Dialect {
 	}
 
 	public String getLimitString(String querySelect, int offset, int limit) {
-		if (offset>0) throw new UnsupportedOperationException("informix has no offset");
+		if ( offset > 0 ) {
+			throw new UnsupportedOperationException( "query result offset is not supported" );
+		}
 		return new StringBuffer( querySelect.length() + 8 )
 				.append( querySelect )
 				.insert( querySelect.toLowerCase().indexOf( "select" ) + 6, " first " + limit )

core/src/main/java/org/hibernate/dialect/IngresDialect.java

@@ -256,7 +256,7 @@ public class IngresDialect extends Dialect {
 	 */
 	public String getLimitString(String querySelect, int offset, int limit) {
 		if ( offset > 0 ) {
-			throw new UnsupportedOperationException( "offset not supported" );
+			throw new UnsupportedOperationException( "query result offset is not supported" );
 		}
 		return new StringBuffer( querySelect.length() + 16 )
 				.append( querySelect )

core/src/main/java/org/hibernate/dialect/MySQLDialect.java

@@ -242,32 +242,6 @@ public class MySQLDialect extends Dialect {
 				.toString();
 	}
 
-	/*
-	 * Temporary, until MySQL fix Connector/J bug
-	 */
-	/*public String getLimitString(String sql, int offset, int limit) {
-		StringBuffer buf = new StringBuffer( sql.length()+20 )
-			.append(sql);
-		if (offset>0) {
-			buf.append(" limit ")
-				.append(offset)
-				.append(", ")
-				.append(limit);
-		}
-		else {
-			buf.append(" limit ")
-				.append(limit);
-		}
-		return buf.toString();
-	}*/
-
-	/*
-	 * Temporary, until MySQL fix Connector/J bug
-	 */
-	/*public boolean supportsVariableLimit() {
-		return false;
-	}*/
-
 	public char closeQuote() {
 		return '`';
 	}

core/src/main/java/org/hibernate/dialect/RDMSOS2200Dialect.java

@@ -314,7 +314,9 @@ public class RDMSOS2200Dialect extends Dialect {
 	}
 
     public String getLimitString(String sql, int offset, int limit) {
-        if (offset>0) throw new UnsupportedOperationException("RDMS does not support paged queries");
+		if ( offset > 0 ) {
+			throw new UnsupportedOperationException( "query result offset is not supported" );
+		}
 		return new StringBuffer( sql.length() + 40 )
 				.append( sql )
 				.append( " fetch first " )

core/src/main/java/org/hibernate/dialect/SQLServerDialect.java

@@ -65,7 +65,7 @@ public class SQLServerDialect extends SybaseDialect {
 
 	public String getLimitString(String querySelect, int offset, int limit) {
 		if ( offset > 0 ) {
-			throw new UnsupportedOperationException( "sql server has no offset" );
+			throw new UnsupportedOperationException( "query result offset is not supported" );
 		}
 		return new StringBuffer( querySelect.length() + 8 )
 				.append( querySelect )

core/src/main/java/org/hibernate/dialect/TimesTenDialect.java

@@ -174,7 +174,7 @@ public class TimesTenDialect extends Dialect {
 
 	public String getLimitString(String querySelect, int offset, int limit) {
 		if ( offset > 0 ) {
-			throw new UnsupportedOperationException( "TimesTen does not support offset" );
+			throw new UnsupportedOperationException( "query result offset is not supported" );
 		}
 		return new StringBuffer( querySelect.length() + 8 )
 				.append( querySelect )

core/src/main/java/org/hibernate/loader/Loader.java

@@ -1521,6 +1521,10 @@ public abstract class Loader {
 		}
 	}
 
+	private int interpretFirstRow(int zeroBasedFirstResult) {
+		return getFactory().getDialect().convertToFirstRowValue( zeroBasedFirstResult );
+	}
+
 	/**
 	 * Should we pre-process the SQL string, adding a dialect-specific
 	 * LIMIT clause.
@@ -1627,7 +1631,7 @@ public abstract class Loader {
 	 * @return The appropriate value to bind into the limit clause.
 	 */
 	private static int getMaxOrLimit(final RowSelection selection, final Dialect dialect) {
-		final int firstRow = getFirstRow( selection );
+		final int firstRow = dialect.convertToFirstRowValue( getFirstRow( selection ) );
 		final int lastRow = selection.getMaxRows().intValue();
 		if ( dialect.useMaxForLimit() ) {
 			return lastRow + firstRow;
@@ -1657,7 +1661,7 @@ public abstract class Loader {
 		if ( !hasMaxRows( selection ) ) {
 			throw new AssertionFailure( "no max results set" );
 		}
-		int firstRow = getFirstRow( selection );
+		int firstRow = interpretFirstRow( getFirstRow( selection ) );
 		int lastRow = getMaxOrLimit( selection, dialect );
 		boolean hasFirstRow = dialect.supportsLimitOffset() && ( firstRow > 0 || dialect.forceLimitUsage() );
 		boolean reverse = dialect.bindLimitParametersInReverseOrder();
@@ -1675,7 +1679,7 @@ public abstract class Loader {
 			final PreparedStatement st,
 			final RowSelection selection) throws SQLException {
 		if ( hasMaxRows( selection ) ) {
-			st.setMaxRows( selection.getMaxRows().intValue() + getFirstRow( selection ) );
+			st.setMaxRows( selection.getMaxRows().intValue() + interpretFirstRow( getFirstRow( selection ) ) );
 		}
 	}
 

testsuite/src/test/java/org/hibernate/test/pagination/DataPoint.hbm.xml

@@ -1,4 +1,27 @@
 <?xml version="1.0"?>
+<!--
+  ~ Hibernate, Relational Persistence for Idiomatic Java
+  ~
+  ~ Copyright (c) 2009, 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
+  -->
 <!DOCTYPE hibernate-mapping PUBLIC
 	"-//Hibernate/Hibernate Mapping DTD 3.0//EN"
 	"http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd">
@@ -12,6 +35,7 @@
 		<id name="id">
 			<generator class="increment"/>
 		</id>
+        <property name="sequence" not-null="true" column="seqval" type="int" />
         <property name="x">
 			<column name="xval" not-null="true" precision="20" scale="19" unique-key="xy"/>
 		</property>

testsuite/src/test/java/org/hibernate/test/pagination/DataPoint.java

@@ -1,4 +1,26 @@
-//$Id: DataPoint.java 7867 2005-08-11 23:35:33Z oneovthafew $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2009, 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.pagination;
 
 import java.math.BigDecimal;
@@ -8,51 +30,78 @@ import java.math.BigDecimal;
  */
 public class DataPoint {
 	private long id;
+	private int sequence;
 	private BigDecimal x;
 	private BigDecimal y;
 	private String description;
-	/**
-	 * @return Returns the description.
-	 */
-	public String getDescription() {
-		return description;
-	}
-	/**
-	 * @param description The description to set.
-	 */
-	public void setDescription(String description) {
-		this.description = description;
-	}
+
 	/**
 	 * @return Returns the id.
 	 */
 	public long getId() {
 		return id;
 	}
+
 	/**
 	 * @param id The id to set.
 	 */
 	public void setId(long id) {
 		this.id = id;
 	}
+
+	/**
+	 * Getter for property 'sequence'.
+	 *
+	 * @return Value for property 'sequence'.
+	 */
+	public int getSequence() {
+		return sequence;
+	}
+
+	/**
+	 * Setter for property 'sequence'.
+	 *
+	 * @param sequence Value to set for property 'sequence'.
+	 */
+	public void setSequence(int sequence) {
+		this.sequence = sequence;
+	}
+
+	/**
+	 * @return Returns the description.
+	 */
+	public String getDescription() {
+		return description;
+	}
+
+	/**
+	 * @param description The description to set.
+	 */
+	public void setDescription(String description) {
+		this.description = description;
+	}
+
 	/**
 	 * @return Returns the x.
 	 */
 	public BigDecimal getX() {
 		return x;
 	}
+
 	/**
 	 * @param x The x to set.
 	 */
 	public void setX(BigDecimal x) {
 		this.x = x;
 	}
+
 	/**
 	 * @return Returns the y.
 	 */
 	public BigDecimal getY() {
 		return y;
 	}
+
 	/**
 	 * @param y The y to set.
 	 */

testsuite/src/test/java/org/hibernate/test/pagination/PaginationTest.java

@@ -1,14 +1,37 @@
-//$Id: PaginationTest.java 10977 2006-12-12 23:28:04Z steve.ebersole@jboss.com $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2009, 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.pagination;
 
 import java.math.BigDecimal;
+import java.util.List;
 
 import junit.framework.Test;
 
 import org.hibernate.Session;
-import org.hibernate.Transaction;
-import org.hibernate.cfg.Configuration;
-import org.hibernate.cfg.Environment;
+import org.hibernate.SQLQuery;
+import org.hibernate.Query;
+import org.hibernate.Criteria;
 import org.hibernate.criterion.Order;
 import org.hibernate.junit.functional.FunctionalTestCase;
 import org.hibernate.junit.functional.FunctionalTestClassTestSuite;
@@ -17,6 +40,7 @@ import org.hibernate.junit.functional.FunctionalTestClassTestSuite;
  * @author Gavin King
  */
 public class PaginationTest extends FunctionalTestCase {
+	public static final int ROWS = 100;
 
 	public PaginationTest(String str) {
 		super(str);
@@ -26,10 +50,6 @@ public class PaginationTest extends FunctionalTestCase {
 		return new String[] { "pagination/DataPoint.hbm.xml" };
 	}
 
-	public void configure(Configuration cfg) {
-		cfg.setProperty(Environment.STATEMENT_BATCH_SIZE, "20");
-	}
-
 	public String getCacheConcurrencyStrategy() {
 		return null;
 	}
@@ -38,39 +58,125 @@ public class PaginationTest extends FunctionalTestCase {
 		return new FunctionalTestClassTestSuite( PaginationTest.class );
 	}
 
-	public void testPagination() {
-		Session s = openSession();
-		Transaction t = s.beginTransaction();		
-		for ( int i=0; i<10; i++ ) {
-			DataPoint dp = new DataPoint();
-			dp.setX( new BigDecimal(i * 0.1d).setScale(19, BigDecimal.ROUND_DOWN) );
-			dp.setY( new BigDecimal( Math.cos( dp.getX().doubleValue() ) ).setScale(19, BigDecimal.ROUND_DOWN) );
-			s.persist(dp);
+	public void testLimit() {
+		if ( ! getDialect().supportsLimit() ) {
+			reportSkip( "Dialect does not support limit" );
+			return;
 		}
-		t.commit();
-		s.close();
 
-		s = openSession();
-		t = s.beginTransaction();
-		int size = s.createSQLQuery("select id, xval, yval, description from DataPoint order by xval, yval")
-			.addEntity(DataPoint.class)
+		prepareTestData();
+
+		Session session = openSession();
+		session.beginTransaction();
+
+		int count;
+
+		count = generateBaseHQLQuery( session )
 				.setMaxResults( 5 )
-			.list().size();
-		assertEquals(size, 5);
-		size = s.createQuery("from DataPoint order by x, y")
-			.setFirstResult(5)
-			.setMaxResults(2)
-			.list().size();
-		assertEquals(size, 2);
-		size = s.createCriteria(DataPoint.class)
-			.addOrder( Order.asc("x") )
-			.addOrder( Order.asc("y") )
-			.setFirstResult(8)
-			.list().size();
-		assertEquals(size, 2);
-		t.commit();
-		s.close();
+				.list()
+				.size();
+		assertEquals( 5, count );
 
+		count = generateBaseCriteria( session )
+				.setMaxResults( 18 )
+				.list()
+				.size();
+		assertEquals( 18, count );
+
+		count = generateBaseSQLQuery( session )
+				.setMaxResults( 13 )
+				.list()
+				.size();
+		assertEquals( 13, count );
+
+		session.getTransaction().commit();
+		session.close();
+
+		cleanupTestData();
+	}
+
+	public void testLimitOffset() {
+		if ( ! getDialect().supportsLimitOffset() ) {
+			reportSkip( "Dialect does not support limit+offset" );
+			return;
+		}
+
+		prepareTestData();
+
+		Session session = openSession();
+		session.beginTransaction();
+
+		List result;
+
+		result = generateBaseHQLQuery( session )
+				.setFirstResult( 0 )
+				.setMaxResults( 20 )
+				.list();
+		assertEquals( 20, result.size() );
+		assertEquals( 0, ( ( DataPoint ) result.get( 0 ) ).getSequence() );
+		assertEquals( 1, ( ( DataPoint ) result.get( 1 ) ).getSequence() );
+
+		result = generateBaseCriteria( session )
+				.setFirstResult( 1 )
+				.setMaxResults( 20 )
+				.list();
+		assertEquals( 20, result.size() );
+		assertEquals( 1, ( ( DataPoint ) result.get( 0 ) ).getSequence() );
+		assertEquals( 2, ( ( DataPoint ) result.get( 1 ) ).getSequence() );
+
+		result = generateBaseCriteria( session )
+				.setFirstResult( 99 )
+				.setMaxResults( Integer.MAX_VALUE )
+				.list();
+		assertEquals( 1, result.size() );
+		assertEquals( 99, ( ( DataPoint ) result.get( 0 ) ).getSequence() );
+
+		session.getTransaction().commit();
+		session.close();
+
+		cleanupTestData();
+	}
+
+	private Query generateBaseHQLQuery(Session session) {
+		return session.createQuery( "select dp from DataPoint dp order by dp.sequence" );
+	}
+
+	private Criteria generateBaseCriteria(Session session) {
+		return session.createCriteria( DataPoint.class )
+				.addOrder( Order.asc( "sequence" ) );
+	}
+
+	private SQLQuery generateBaseSQLQuery(Session session) {
+		return session.createSQLQuery( "select id, seqval, xval, yval, description from DataPoint order by seqval" )
+				.addEntity( DataPoint.class );
+	}
+
+	private void prepareTestData() {
+		Session session = openSession();
+		session.beginTransaction();
+		for ( int i = 0; i < ROWS; i++ ) {
+			DataPoint dataPoint = new DataPoint();
+			dataPoint.setSequence( i );
+			dataPoint.setDescription( "data point #" + i );
+			BigDecimal x = new BigDecimal( i * 0.1d ).setScale( 19, BigDecimal.ROUND_DOWN );
+			dataPoint.setX( x );
+			dataPoint.setY( new BigDecimal( Math.cos( x.doubleValue() ) ).setScale( 19, BigDecimal.ROUND_DOWN ) );
+			session.save( dataPoint );
+		}
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	private void cleanupTestData() {
+		Session session = openSession();
+		session.beginTransaction();
+		session.createQuery( "delete DataPoint" ).executeUpdate();
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	private void reportSkip(String message) {
+		reportSkip( message, "pagination support" );
 	}
 }