Return last N results by sortkey when using sortkey_after
In the SQL index implementation, sortkey_after queries are always rewritten to use one of the ChangeAccess.*Prev() methods, which order results by sort key ascending. Update the Lucene implementations to match this sort order. Specify in the ChangeIndex javadoc that this sort order is required in such cases, since other code in QueryProcessor depends on this assumption. Change-Id: Ie35d2c6bd07963db6d33145a83493d12c6a4289d
This commit is contained in:
@@ -42,6 +42,7 @@ import com.google.gerrit.server.query.Predicate;
|
|||||||
import com.google.gerrit.server.query.QueryParseException;
|
import com.google.gerrit.server.query.QueryParseException;
|
||||||
import com.google.gerrit.server.query.change.ChangeData;
|
import com.google.gerrit.server.query.change.ChangeData;
|
||||||
import com.google.gerrit.server.query.change.ChangeDataSource;
|
import com.google.gerrit.server.query.change.ChangeDataSource;
|
||||||
|
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.gwtorm.server.ResultSet;
|
import com.google.gwtorm.server.ResultSet;
|
||||||
import com.google.inject.assistedinject.Assisted;
|
import com.google.inject.assistedinject.Assisted;
|
||||||
@@ -249,7 +250,8 @@ public class LuceneChangeIndex implements ChangeIndex {
|
|||||||
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
|
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
|
||||||
indexes.add(closedIndex);
|
indexes.add(closedIndex);
|
||||||
}
|
}
|
||||||
return new QuerySource(indexes, QueryBuilder.toQuery(p), limit);
|
return new QuerySource(indexes, QueryBuilder.toQuery(p), limit,
|
||||||
|
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(p));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -270,11 +272,14 @@ public class LuceneChangeIndex implements ChangeIndex {
|
|||||||
private final List<SubIndex> indexes;
|
private final List<SubIndex> indexes;
|
||||||
private final Query query;
|
private final Query query;
|
||||||
private final int limit;
|
private final int limit;
|
||||||
|
private final boolean reverse;
|
||||||
|
|
||||||
public QuerySource(List<SubIndex> indexes, Query query, int limit) {
|
private QuerySource(List<SubIndex> indexes, Query query, int limit,
|
||||||
|
boolean reverse) {
|
||||||
this.indexes = indexes;
|
this.indexes = indexes;
|
||||||
this.query = query;
|
this.query = query;
|
||||||
this.limit = limit;
|
this.limit = limit;
|
||||||
|
this.reverse = reverse;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -295,11 +300,14 @@ public class LuceneChangeIndex implements ChangeIndex {
|
|||||||
@Override
|
@Override
|
||||||
public ResultSet<ChangeData> read() throws OrmException {
|
public ResultSet<ChangeData> read() throws OrmException {
|
||||||
IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
|
IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
|
||||||
|
@SuppressWarnings("deprecation")
|
||||||
Sort sort = new Sort(
|
Sort sort = new Sort(
|
||||||
new SortField(
|
new SortField(
|
||||||
ChangeField.UPDATED.getName(),
|
ChangeField.SORTKEY.getName(),
|
||||||
SortField.Type.INT,
|
SortField.Type.LONG,
|
||||||
true /* descending */));
|
// Standard order is descending by sort key, unless reversed due
|
||||||
|
// to a sortkey_before predicate.
|
||||||
|
!reverse));
|
||||||
try {
|
try {
|
||||||
TopDocs[] hits = new TopDocs[indexes.size()];
|
TopDocs[] hits = new TopDocs[indexes.size()];
|
||||||
for (int i = 0; i < indexes.size(); i++) {
|
for (int i = 0; i < indexes.size(); i++) {
|
||||||
|
|||||||
@@ -132,7 +132,11 @@ public interface ChangeIndex {
|
|||||||
* or NOT predicates as internal nodes, and {@link IndexPredicate}s as
|
* or NOT predicates as internal nodes, and {@link IndexPredicate}s as
|
||||||
* leaves.
|
* leaves.
|
||||||
* @param limit maximum number of results to return.
|
* @param limit maximum number of results to return.
|
||||||
* @return a source of documents matching the predicate.
|
* @return a source of documents matching the predicate. Documents must be
|
||||||
|
* returned in descending sort key order, unless a {@code sortkey_after}
|
||||||
|
* predicate (with a cut point not at {@link Long#MAX_VALUE}) is provided,
|
||||||
|
* in which case the source should return documents in ascending sort key
|
||||||
|
* order starting from the sort key cut point.
|
||||||
*
|
*
|
||||||
* @throws QueryParseException if the predicate could not be converted to an
|
* @throws QueryParseException if the predicate could not be converted to an
|
||||||
* indexed data source.
|
* indexed data source.
|
||||||
|
|||||||
@@ -121,6 +121,12 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
|||||||
return ((IntPredicate<?>) find(p, IntPredicate.class, FIELD_LIMIT)).intValue();
|
return ((IntPredicate<?>) find(p, IntPredicate.class, FIELD_LIMIT)).intValue();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static boolean hasNonTrivialSortKeyAfter(Predicate<ChangeData> p) {
|
||||||
|
SortKeyPredicate after =
|
||||||
|
(SortKeyPredicate) find(p, SortKeyPredicate.class, "sortkey_after");
|
||||||
|
return after != null && after.getMaxValue() > 0;
|
||||||
|
}
|
||||||
|
|
||||||
public static boolean hasSortKey(Predicate<ChangeData> p) {
|
public static boolean hasSortKey(Predicate<ChangeData> p) {
|
||||||
return find(p, SortKeyPredicate.class, "sortkey_after") != null
|
return find(p, SortKeyPredicate.class, "sortkey_after") != null
|
||||||
|| find(p, SortKeyPredicate.class, "sortkey_before") != null;
|
|| find(p, SortKeyPredicate.class, "sortkey_before") != null;
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ import com.google.gerrit.server.query.Predicate;
|
|||||||
import com.google.gerrit.server.query.QueryParseException;
|
import com.google.gerrit.server.query.QueryParseException;
|
||||||
import com.google.gerrit.server.query.change.ChangeData;
|
import com.google.gerrit.server.query.change.ChangeData;
|
||||||
import com.google.gerrit.server.query.change.ChangeDataSource;
|
import com.google.gerrit.server.query.change.ChangeDataSource;
|
||||||
|
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.gwtorm.server.ResultSet;
|
import com.google.gwtorm.server.ResultSet;
|
||||||
|
|
||||||
@@ -204,7 +205,8 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
|
|||||||
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
|
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
|
||||||
indexes.add(closedIndex);
|
indexes.add(closedIndex);
|
||||||
}
|
}
|
||||||
return new QuerySource(indexes, QueryBuilder.toQuery(p), limit);
|
return new QuerySource(indexes, QueryBuilder.toQuery(p), limit,
|
||||||
|
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(p));
|
||||||
}
|
}
|
||||||
|
|
||||||
private void commit(SolrServer server) throws IOException {
|
private void commit(SolrServer server) throws IOException {
|
||||||
@@ -219,7 +221,9 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
|
|||||||
private final List<SolrServer> indexes;
|
private final List<SolrServer> indexes;
|
||||||
private final SolrQuery query;
|
private final SolrQuery query;
|
||||||
|
|
||||||
public QuerySource(List<SolrServer> indexes, Query q, int limit) {
|
@SuppressWarnings("deprecation")
|
||||||
|
public QuerySource(List<SolrServer> indexes, Query q, int limit,
|
||||||
|
boolean reverse) {
|
||||||
this.indexes = indexes;
|
this.indexes = indexes;
|
||||||
|
|
||||||
query = new SolrQuery(q.toString());
|
query = new SolrQuery(q.toString());
|
||||||
@@ -227,8 +231,8 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
|
|||||||
query.setParam("rows", Integer.toString(limit));
|
query.setParam("rows", Integer.toString(limit));
|
||||||
query.setFields(ID_FIELD);
|
query.setFields(ID_FIELD);
|
||||||
query.setSort(
|
query.setSort(
|
||||||
ChangeField.UPDATED.getName(),
|
ChangeField.SORTKEY.getName(),
|
||||||
SolrQuery.ORDER.desc);
|
!reverse ? SolrQuery.ORDER.desc : SolrQuery.ORDER.asc);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
Reference in New Issue
Block a user