Skip to content

Remove the sort from query before executing count. #1194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}

Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,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;
Expand Down Expand Up @@ -448,7 +449,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());
Expand Down