Remove the sort from query before executing count. (#1194)

Also creates the query with skip and limit from any Pageable parameter instead of applying it later.
Closes #1191.

Author: mikereiche

src/main/java/org/springframework/data/couchbase/core/query/Query.java

@@ -136,7 +136,9 @@ public Query with(final Pageable pageable) {
 		}
 		this.limit = pageable.getPageSize();
 		this.skip = pageable.getOffset();
-		return with(pageable.getSort());
+		if(!this.sort.equals(pageable.getSort()))
+			this.sort.and(pageable.getSort());
+		return this;
 	}
 
 	/**
@@ -174,6 +176,11 @@ public Query with(final Sort sort) {
 		return this;
 	}
 
+	public Query withoutSort(){
+		this.sort = Sort.unsorted();
+		return this;
+	}
+
 	public void appendSkipAndLimit(final StringBuilder sb) {
 		if (limit > 0) {
 			sb.append(" LIMIT ").append(limit);
@@ -287,8 +294,10 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String coll
 		appendString(statement, n1ql.selectEntity); // select ...
 		appendWhereString(statement, n1ql.filter); // typeKey = typeValue
 		appendWhere(statement, new int[] { 0 }, template.getConverter()); // criteria on this Query
-		appendSort(statement);
-		appendSkipAndLimit(statement);
+		if(!isCount){
+			appendSort(statement);
+			appendSkipAndLimit(statement);
+		}
 		return statement.toString();
 	}
 

src/main/java/org/springframework/data/couchbase/repository/query/CouchbaseQueryExecution.java

@@ -19,8 +19,8 @@
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.couchbase.core.CouchbaseOperations;
-import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.TerminatingFindByQuery;
 import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.ExecutableFindByQuery;
+import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.TerminatingFindByQuery;
 import org.springframework.data.couchbase.core.query.Query;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
@@ -112,7 +112,7 @@ public SlicedExecution(ExecutableFindByQuery find, Pageable pageable) {
 		public Object execute(Query query, Class<?> type, String collection) {
 			int pageSize = pageable.getPageSize();
 			// Apply Pageable but tweak limit to peek into next page
-			Query modifiedQuery = query.with(pageable).limit(pageSize + 1);
+			Query modifiedQuery = query.skip(pageable.getOffset()).limit(pageSize + 1);
 			List result = find.matching(modifiedQuery).all();
 			boolean hasNext = result.size() > pageSize;
 			return new SliceImpl<Object>(hasNext ? result.subList(0, pageSize) : result, pageable, hasNext);
@@ -142,14 +142,12 @@ public PagedExecution(ExecutableFindByQuery<?> operation, Pageable pageable) {
 		public Object execute(Query query, Class<?> type, String collection) {
 			int overallLimit = 0; // query.getLimit();
 			TerminatingFindByQuery<?> matching = operation.matching(query);
-			// Apply raw pagination
-			query.with(pageable);
 			// Adjust limit if page would exceed the overall limit
 			if (overallLimit != 0 && pageable.getOffset() + pageable.getPageSize() > overallLimit) {
 				query.limit((int) (overallLimit - pageable.getOffset()));
 			}
 			return PageableExecutionUtils.getPage(matching.all(), pageable, () -> {
-				long count = operation.matching(query.skip(-1).limit(-1)).count();
+				long count = operation.matching(query.skip(-1).limit(-1).withoutSort()).count();
 				return overallLimit != 0 ? Math.min(count, overallLimit) : count;
 			});
 		}

src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java

@@ -22,13 +22,15 @@
 import static org.springframework.data.couchbase.core.query.QueryCriteria.where;
 
 import java.util.Iterator;
+import java.util.Optional;
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
 import org.springframework.data.couchbase.core.query.N1QLExpression;
 import org.springframework.data.couchbase.core.query.Query;
 import org.springframework.data.couchbase.core.query.QueryCriteria;
+import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.mapping.context.MappingContext;
@@ -37,6 +39,7 @@
 import org.springframework.data.repository.query.parser.AbstractQueryCreator;
 import org.springframework.data.repository.query.parser.Part;
 import org.springframework.data.repository.query.parser.PartTree;
+import org.springframework.util.Assert;
 
 /**
  * @author Michael Nitschinger
@@ -72,12 +75,24 @@ protected QueryCriteria create(final Part part, final Iterator<Object> iterator)
 		return from(part, property, where(addMetaIfRequired(bucketName, path, property)), iterator);
 	}
 
+	@Override
+	public Query createQuery() {
+		Query q = this.createQuery((Optional.of(this.accessor).map(ParameterAccessor::getSort).orElse(Sort.unsorted())));
+		Pageable pageable = accessor.getPageable();
+		if(pageable.isPaged()) {
+			q.skip(pageable.getOffset());
+			q.limit(pageable.getPageSize());
+		}
+		return q;
+	}
+
 	static Converter<? super CouchbasePersistentProperty, String> cvtr = new MyConverter();
 
 	static class MyConverter implements Converter<CouchbasePersistentProperty, String> {
 		@Override
 		public String convert(CouchbasePersistentProperty source) {
-				return new StringBuilder(source.getFieldName().length()+2).append("`").append(source.getFieldName()).append("`").toString();
+			return new StringBuilder(source.getFieldName().length() + 2).append("`").append(source.getFieldName()).append("`")
+					.toString();
 		}
 	}
 
@@ -90,7 +105,7 @@ protected QueryCriteria and(final Part part, final QueryCriteria base, final Ite
 		PersistentPropertyPath<CouchbasePersistentProperty> path = context.getPersistentPropertyPath(part.getProperty());
 		CouchbasePersistentProperty property = path.getLeafProperty();
 
-		return from(part, property, base.and(addMetaIfRequired(bucketName,path, property)), iterator);
+		return from(part, property, base.and(addMetaIfRequired(bucketName, path, property)), iterator);
 	}
 
 	@Override
@@ -100,7 +115,8 @@ protected QueryCriteria or(QueryCriteria base, QueryCriteria criteria) {
 
 	@Override
 	protected Query complete(QueryCriteria criteria, Sort sort) {
-		return (criteria == null ? new Query() : new Query().addCriteria(criteria)).with(sort);
+		Query q = (criteria == null ? new Query() : new Query().addCriteria(criteria)).with(sort);
+		return q;
 	}
 
 	private QueryCriteria from(final Part part, final CouchbasePersistentProperty property, final QueryCriteria criteria,
@@ -166,8 +182,7 @@ private QueryCriteria from(final Part part, final CouchbasePersistentProperty pr
 		}
 	}
 
-	public static N1QLExpression addMetaIfRequired(
-			String bucketName,
+	public static N1QLExpression addMetaIfRequired(String bucketName,
 			final PersistentPropertyPath<CouchbasePersistentProperty> persistentPropertyPath,
 			final CouchbasePersistentProperty property) {
 		if (property.isIdProperty()) {

src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java

@@ -76,6 +76,7 @@
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
@@ -449,7 +450,7 @@ void count() {
 			Long count = airportRepository.countFancyExpression(asList("JFK"), asList("jfk"), false);
 			assertEquals(1, count);
 
-			Pageable pageable = PageRequest.of(0, 2);
+			Pageable pageable = PageRequest.of(0, 2).withSort(Sort.by("iata"));
 			Page<Airport> aPage = airportRepository.findAllByIataNot("JFK", pageable);
 			assertEquals(iatas.length - 1, aPage.getTotalElements());
 			assertEquals(pageable.getPageSize(), aPage.getContent().size());