Fix paging errors with descending keys

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java

@@ -27,17 +27,21 @@ import java.util.List;
 import java.util.ArrayList;
 
 /**
- * Abstract SQL Paging Query Provider to serve as a base class for all provided SQL paging query providers.
- *
- * Any implementation must provide a way to specify the select clause, from clause and optionally a where clause. 
- * In addition a way to specify a single column sort key must also be provided. This sort key will be used to 
- * provide the paging functionality. It is recommended that there should be an index for the sort key to provide 
- * better performance.
- *
- * Provides properties and preparation for the mandatory "selectClause" and "fromClause" as well as for the
- * optional "whereClause".  Also provides property for the mandatory "sortKey".
- *
+ * Abstract SQL Paging Query Provider to serve as a base class for all provided
+ * SQL paging query providers.
+ * 
+ * Any implementation must provide a way to specify the select clause, from
+ * clause and optionally a where clause. In addition a way to specify a single
+ * column sort key must also be provided. This sort key will be used to provide
+ * the paging functionality. It is recommended that there should be an index for
+ * the sort key to provide better performance.
+ * 
+ * Provides properties and preparation for the mandatory "selectClause" and
+ * "fromClause" as well as for the optional "whereClause". Also provides
+ * property for the mandatory "sortKey".
+ * 
  * @author Thomas Risberg
+ * @author Dave Syer
  * @since 2.0
  */
 public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvider {
@@ -50,11 +54,12 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 
 	private String sortKey;
 
+	private boolean ascending = true;
+
 	private int parameterCount;
 
 	private boolean usingNamedParameters;
 
-
 	/**
 	 * @param selectClause SELECT clause part of SQL query string
 	 */
@@ -63,7 +68,7 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 	}
 
 	/**
-	 *
+	 * 
 	 * @return SQL SELECT clause part of SQL query string
 	 */
 	protected String getSelectClause() {
@@ -78,7 +83,7 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 	}
 
 	/**
-	 *
+	 * 
 	 * @return SQL FROM clause part of SQL query string
 	 */
 	protected String getFromClause() {
@@ -93,12 +98,12 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 			this.whereClause = removeKeyWord("where", whereClause);
 		}
 		else {
-			this.whereClause = null;			
+			this.whereClause = null;
 		}
 	}
 
 	/**
-	 *
+	 * 
 	 * @return SQL WHERE clause part of SQL query string
 	 */
 	protected String getWhereClause() {
@@ -113,14 +118,32 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 	}
 
 	/**
-	 *
+	 * Set the flag that signals that the sort key is applied ascending (default
+	 * true).
+	 * 
+	 * @param ascending the ascending value to set
+	 */
+	public void setAscending(boolean ascending) {
+		this.ascending = ascending;
+	}
+
+	/**
+	 * Get the flag that signals that the sort key is applied ascending.
+	 * 
+	 * @return the ascending flag
+	 */
+	public boolean isAscending() {
+		return ascending;
+	}
+
+	/**
+	 * 
 	 * @return sortKey key to use to sort and limit page content
 	 */
 	protected String getSortKey() {
 		return sortKey;
 	}
 
-
 	public int getParameterCount() {
 		return parameterCount;
 	}
@@ -130,9 +153,9 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 	}
 
 	/**
-	 * The sort key placeholder will vary depending on whether named parameters or traditional placeholders
-	 * are used in query strings.
-	 *
+	 * The sort key placeholder will vary depending on whether named parameters
+	 * or traditional placeholders are used in query strings.
+	 * 
 	 * @return place holder for sortKey.
 	 */
 	protected String getSortKeyPlaceHolder() {
@@ -158,41 +181,42 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi
 		parameterCount = JdbcParameterUtils.countParameterPlaceholders(sql.toString(), namedParameters);
 		if (namedParameters.size() > 0) {
 			if (parameterCount != namedParameters.size()) {
-				throw new InvalidDataAccessApiUsageException("You can't use both named parameters and classic \"?\" placeholders: " + sql);
+				throw new InvalidDataAccessApiUsageException(
+						"You can't use both named parameters and classic \"?\" placeholders: " + sql);
 			}
 			usingNamedParameters = true;
 		}
 	}
 
 	/**
-	 * Method generating the query string to be used for retrieving the first page.
-	 * This method must be implemented in sub classes.
-	 *
+	 * Method generating the query string to be used for retrieving the first
+	 * page. This method must be implemented in sub classes.
+	 * 
 	 * @param pageSize number of rows to read per page
 	 * @return query string
 	 */
 	public abstract String generateFirstPageQuery(int pageSize);
 
 	/**
-	 * Method generating the query string to be used for retrieving the pages following the first page.
-	 * This method must be implemented in sub classes.
-	 *
+	 * Method generating the query string to be used for retrieving the pages
+	 * following the first page. This method must be implemented in sub classes.
+	 * 
 	 * @param pageSize number of rows to read per page
 	 * @return query string
 	 */
 	public abstract String generateRemainingPagesQuery(int pageSize);
 
 	/**
-	 * Method generating the query string to be used for jumping to a specific item position.  
-	 * This method must be implemented in sub classes.
-	 *
+	 * Method generating the query string to be used for jumping to a specific
+	 * item position. This method must be implemented in sub classes.
+	 * 
 	 * @param itemIndex the index of the item to jump to
 	 * @param pageSize number of rows to read per page
 	 * @return query string
 	 */
 	public abstract String generateJumpToItemQuery(int itemIndex, int pageSize);
 
-	private String removeKeyWord(String keyWord,  String clause) {
+	private String removeKeyWord(String keyWord, String clause) {
 		String temp = clause.trim();
 		String keyWordString = keyWord + " ";
 		if (temp.toLowerCase().startsWith(keyWordString) && temp.length() > keyWordString.length()) {

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/HsqlPagingQueryProvider.java

@@ -42,6 +42,7 @@ public class HsqlPagingQueryProvider extends AbstractSqlPagingQueryProvider {
 	public String generateJumpToItemQuery(int itemIndex, int pageSize) {
 		int page = itemIndex / pageSize;
 		int offset = (page * pageSize) - 1;
+		offset = offset<0 ? 0 : offset;
 
 		String topClause = new StringBuilder().append("LIMIT ").append(offset).append(" 1").toString();
 		return SqlPagingQueryUtils.generateTopJumpToQuery(this, topClause);

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/MySqlPagingQueryProvider.java

@@ -44,6 +44,7 @@ public class MySqlPagingQueryProvider extends AbstractSqlPagingQueryProvider {
 	public String generateJumpToItemQuery(int itemIndex, int pageSize) {
 		int page = itemIndex / pageSize;
 		int offset = (page * pageSize) - 1;
+		offset = offset<0 ? 0 : offset;
 
 		String limitClause = new StringBuilder().append("LIMIT ").append(offset).append(", 1").toString();
 		return SqlPagingQueryUtils.generateLimitJumpToQuery(this, limitClause);

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/PostgresPagingQueryProvider.java

@@ -42,6 +42,7 @@ public class PostgresPagingQueryProvider extends AbstractSqlPagingQueryProvider
 	public String generateJumpToItemQuery(int itemIndex, int pageSize) {
 		int page = itemIndex / pageSize;
 		int offset = (page * pageSize) - 1;
+		offset = offset<0 ? 0 : offset;
 
 		String limitClause = new StringBuilder().append("LIMIT ").append(offset).append(" 1").toString();
 		return SqlPagingQueryUtils.generateLimitJumpToQuery(this, limitClause);

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/SqlPagingQueryProviderFactoryBean.java

@@ -59,6 +59,8 @@ public class SqlPagingQueryProviderFactoryBean implements FactoryBean {
 	private String sortKey;
 
 	private Map<DatabaseType, AbstractSqlPagingQueryProvider> providers = new HashMap<DatabaseType, AbstractSqlPagingQueryProvider>();
+
+	private boolean ascending;
 	{
 		providers.put(DB2, new Db2PagingQueryProvider());
 		providers.put(DB2ZOS, new Db2PagingQueryProvider());
@@ -113,6 +115,13 @@ public class SqlPagingQueryProviderFactoryBean implements FactoryBean {
 		this.sortKey = sortKey;
 	}
 
+	/**
+	 * @param ascending
+	 */
+	public void setAscending(boolean ascending) {
+		this.ascending = ascending;	
+	}
+
 	/**
 	 * Get a {@link PagingQueryProvider} instance using the provided properties
 	 * and appropriate for the given database type.
@@ -137,6 +146,7 @@ public class SqlPagingQueryProviderFactoryBean implements FactoryBean {
 		provider.setFromClause(fromClause);
 		provider.setWhereClause(whereClause);
 		provider.setSortKey(sortKey);
+		provider.setAscending(ascending);
 		if (StringUtils.hasText(selectClause)) {
 			provider.setSelectClause(selectClause);
 		}
@@ -163,4 +173,5 @@ public class SqlPagingQueryProviderFactoryBean implements FactoryBean {
 	public boolean isSingleton() {
 		return true;
 	}
+
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/SqlPagingQueryUtils.java

@@ -17,65 +17,79 @@
 package org.springframework.batch.item.database.support;
 
 /**
- * Utility class that generates the actual SQL statements used by query providers.
- *
+ * Utility class that generates the actual SQL statements used by query
+ * providers.
+ * 
  * @author Thomas Risberg
+ * @author Dave Syer
  * @since 2.0
  */
 public class SqlPagingQueryUtils {
 
 	/**
 	 * Generate SQL query string using a LIMIT clause
-	 *
-	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the implementation specifics
-	 * @param remainingPageQuery is this query for the ramining pages (true) as opposed to the first page (false)
-	 * @param limitClause the implementation specific limit clause to be used 
+	 * 
+	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the
+	 * implementation specifics
+	 * @param remainingPageQuery is this query for the ramining pages (true) as
+	 * opposed to the first page (false)
+	 * @param limitClause the implementation specific limit clause to be used
 	 * @return the generated query
 	 */
-	public static String generateLimitSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery, String limitClause) {
+	public static String generateLimitSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery,
+			String limitClause) {
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT ").append(provider.getSelectClause());
 		sql.append(" FROM ").append(provider.getFromClause());
 		buildWhereClause(provider, remainingPageQuery, sql);
-		sql.append(" ORDER BY ").append(provider.getSortKey()).append(" ASC ");
-		sql.append(limitClause);
-		
+		sql.append(" ORDER BY ").append(provider.getSortKey());
+		buildAscendingClause(provider, sql);
+		sql.append(" " + limitClause);
+
 		return sql.toString();
 	}
 
 	/**
 	 * Generate SQL query string using a TOP clause
-	 *
-	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the implementation specifics
-	 * @param remainingPageQuery is this query for the ramining pages (true) as opposed to the first page (false)
+	 * 
+	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the
+	 * implementation specifics
+	 * @param remainingPageQuery is this query for the ramining pages (true) as
+	 * opposed to the first page (false)
 	 * @param topClause the implementation specific top clause to be used
 	 * @return the generated query
 	 */
-	public static String generateTopSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery, String topClause) {
+	public static String generateTopSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery,
+			String topClause) {
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT ").append(topClause).append(" ").append(provider.getSelectClause());
 		sql.append(" FROM ").append(provider.getFromClause());
 		buildWhereClause(provider, remainingPageQuery, sql);
-		sql.append(" ORDER BY ").append(provider.getSortKey()).append(" ASC");
+		sql.append(" ORDER BY ").append(provider.getSortKey());
+		buildAscendingClause(provider, sql);
 
 		return sql.toString();
 	}
 
 	/**
 	 * Generate SQL query string using a ROW_NUM condition
-	 *
-	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the implementation specifics
-	 * @param remainingPageQuery is this query for the ramining pages (true) as opposed to the first page (false)
+	 * 
+	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the
+	 * implementation specifics
+	 * @param remainingPageQuery is this query for the ramining pages (true) as
+	 * opposed to the first page (false)
 	 * @param rowNumClause the implementation specific row num clause to be used
 	 * @return the generated query
 	 */
-	public static String generateRowNumSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery, String rowNumClause) {
+	public static String generateRowNumSqlQuery(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery,
+			String rowNumClause) {
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT ").append(provider.getSelectClause());
 		sql.append(" FROM ").append(provider.getFromClause());
 		buildWhereClause(provider, remainingPageQuery, sql);
 		sql.append(provider.getWhereClause() != null || remainingPageQuery ? " AND " : " WHERE ").append(rowNumClause);
-		sql.append(" ORDER BY ").append(provider.getSortKey()).append(" ASC");
+		sql.append(" ORDER BY ").append(provider.getSortKey());
+		buildAscendingClause(provider, sql);
 
 		return sql.toString();
 
@@ -83,8 +97,9 @@ public class SqlPagingQueryUtils {
 
 	/**
 	 * Generate SQL query string using a LIMIT clause
-	 *
-	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the implementation specifics
+	 * 
+	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the
+	 * implementation specifics
 	 * @param limitClause the implementation specific top clause to be used
 	 * @return the generated query
 	 */
@@ -93,16 +108,18 @@ public class SqlPagingQueryUtils {
 		sql.append("SELECT ").append(provider.getSortKey()).append(" AS SORT_KEY");
 		sql.append(" FROM ").append(provider.getFromClause());
 		sql.append(provider.getWhereClause() == null ? "" : " WHERE " + provider.getWhereClause());
-		sql.append(" ORDER BY ").append(provider.getSortKey()).append(" ASC ");
-		sql.append( limitClause);
+		sql.append(" ORDER BY ").append(provider.getSortKey());
+		buildAscendingClause(provider, sql);
+		sql.append(" " + limitClause);
 
 		return sql.toString();
 	}
 
 	/**
 	 * Generate SQL query string using a TOP clause
-	 *
-	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the implementation specifics
+	 * 
+	 * @param provider {@link AbstractSqlPagingQueryProvider} providing the
+	 * implementation specifics
 	 * @param topClause the implementation specific top clause to be used
 	 * @return the generated query
 	 */
@@ -111,19 +128,37 @@ public class SqlPagingQueryUtils {
 		sql.append("SELECT ").append(topClause).append(" ").append(provider.getSortKey()).append(" AS SORT_KEY");
 		sql.append(" FROM ").append(provider.getFromClause());
 		sql.append(provider.getWhereClause() == null ? "" : " WHERE " + provider.getWhereClause());
-		sql.append(" ORDER BY ").append(provider.getSortKey()).append(" ASC");
+		sql.append(" ORDER BY ").append(provider.getSortKey());
+		buildAscendingClause(provider, sql);
 
 		return sql.toString();
 	}
 
-	private static void buildWhereClause(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery, StringBuilder sql) {
+	private static void buildAscendingClause(AbstractSqlPagingQueryProvider provider, StringBuilder sql) {
+		if (provider.isAscending()) {
+			sql.append(" ASC");
+		}
+		else {
+			sql.append(" DESC");
+		}
+	}
+
+	private static void buildWhereClause(AbstractSqlPagingQueryProvider provider, boolean remainingPageQuery,
+			StringBuilder sql) {
 		if (remainingPageQuery) {
 			sql.append(" WHERE ");
 			if (provider.getWhereClause() != null) {
 				sql.append(provider.getWhereClause());
 				sql.append(" AND ");
 			}
-			sql.append(provider.getSortKey()).append(" > ").append(provider.getSortKeyPlaceHolder());
+			sql.append(provider.getSortKey());
+			if (provider.isAscending()) {
+				sql.append(" > ");
+			}
+			else {
+				sql.append(" < ");
+			}
+			sql.append(provider.getSortKeyPlaceHolder());
 		}
 		else {
 			sql.append(provider.getWhereClause() == null ? "" : " WHERE " + provider.getWhereClause());

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/SqlWindowingPagingQueryProvider.java

@@ -30,7 +30,9 @@ public class SqlWindowingPagingQueryProvider extends AbstractSqlPagingQueryProvi
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT * FROM ( ");
 		sql.append("SELECT ").append(getSelectClause()).append(", ");
-		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey()).append(" ASC) AS ROW_NUMBER");
+		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey());
+		buildAscendingClause(sql);
+		sql.append(") AS ROW_NUMBER");
 		sql.append(" FROM ").append(getFromClause()).append(getWhereClause() == null ? "" : " WHERE " + getWhereClause());
 		sql.append(") WHERE ROW_NUMBER <= ").append(pageSize);
 
@@ -42,14 +44,23 @@ public class SqlWindowingPagingQueryProvider extends AbstractSqlPagingQueryProvi
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT * FROM ( ");
 		sql.append("SELECT ").append(getSelectClause()).append(", ");
-		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey()).append(" ASC) AS ROW_NUMBER");
+		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey());
+		buildAscendingClause(sql);
+		sql.append(") AS ROW_NUMBER");
 		sql.append(" FROM ").append(getFromClause());
 		sql.append(" WHERE ");
 		if (getWhereClause() != null) {
 			sql.append(getWhereClause());
 			sql.append(" AND ");
 		}
-		sql.append(getSortKey()).append(" > ").append(getSortKeyPlaceHolder());
+		sql.append(getSortKey());
+		if (isAscending()) {
+			sql.append(" > ");
+		}
+		else {
+			sql.append(" < ");
+		}
+		sql.append(getSortKeyPlaceHolder());
 		sql.append(") WHERE ROW_NUMBER <= ").append(pageSize);
 
 		return sql.toString();
@@ -59,15 +70,29 @@ public class SqlWindowingPagingQueryProvider extends AbstractSqlPagingQueryProvi
 	public String generateJumpToItemQuery(int itemIndex, int pageSize) {
 		int page = itemIndex / pageSize;
 		int lastRowNum = (page * pageSize);
+		if (lastRowNum<=0) {
+			lastRowNum = 1;
+		}
 
 		StringBuilder sql = new StringBuilder();
 		sql.append("SELECT SORT_KEY FROM ( ");
 		sql.append("SELECT ").append(getSortKey()).append(" AS SORT_KEY, ");
-		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey()).append(" ASC) AS ROW_NUMBER");
+		sql.append("ROW_NUMBER() OVER (ORDER BY ").append(getSortKey());
+		buildAscendingClause(sql);
+		sql.append(") AS ROW_NUMBER");
 		sql.append(" FROM ").append(getFromClause()).append(getWhereClause() == null ? "" : " WHERE " + getWhereClause());
 		sql.append(") WHERE ROW_NUMBER = ").append(lastRowNum);
 
 		return sql.toString();
 	}
 
+	private void buildAscendingClause(StringBuilder sql) {
+		if (isAscending()) {
+			sql.append(" ASC");
+		}
+		else {
+			sql.append(" DESC");
+		}
+	}
+
 }