Fix limit handling in QueryProcessor

This was a mess, and hasn't been working for a long time. The test for
_more_changes added in this change does not pass prior to this change.

The only way we ever communicated that extra results were present out
of QueryProcessor was by including one more result. But different
pieces of the code were rarely clear on whether they should be
expecting an extra result or not. Moreover, there were three different
limits that were considered, and they were all given unhelpful names
like "limit" and "maxLimit."

Improving this situation substantially requires doing something like
what the end result is from the REST API: including for each list of
results a boolean indicating whether there are more results available.
Do this with a QueryResult class encapsulating these fields. This
class also includes the original query string and rewritten predicate
intermediates, which can help with debugging.

After this change, the semantics of the limits should be more clear.
The limit computation uses more helpful variable names, and enforces
that we always request a limit of n+1 from the secondary index.
However, this extra result never escapes to callers in a QueryResult
from QueryProcessor.

This also required changes to the query rewriter to replace all limit
predicates within the query with a fixed value that is always
different from any limit predicate in the original query. This may
actually make IndexRewriteTest a little easier to understand, as prior
to this change a different limit would appear in the IndexChangedQuery
portions than at the top level.

Change-Id: I4fcf7db0136fd77dbe91b20258ae80a9797a1af9

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java

@@ -104,6 +104,7 @@ import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
+import com.google.gerrit.server.query.change.QueryResult;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -252,10 +253,16 @@ public class ChangeJson {
     return format(cd, Optional.of(rsrc.getPatchSet().getId()));
   }
 
-  public List<List<ChangeInfo>> formatList2(List<List<ChangeData>> in)
+  public List<List<ChangeInfo>> formatQueryResults(List<QueryResult> in)
       throws OrmException {
     accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
-    Iterable<ChangeData> all = Iterables.concat(in);
+    Iterable<ChangeData> all = FluentIterable.from(in)
+        .transformAndConcat(new Function<QueryResult, List<ChangeData>>() {
+          @Override
+          public List<ChangeData> apply(QueryResult in) {
+            return in.changes();
+          }
+        });
     ChangeData.ensureChangeLoaded(all);
     if (has(ALL_REVISIONS)) {
       ChangeData.ensureAllPatchSetsLoaded(all);
@@ -270,8 +277,12 @@ public class ChangeJson {
 
     List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size());
     Map<Change.Id, ChangeInfo> out = Maps.newHashMap();
-    for (List<ChangeData> changes : in) {
+    for (QueryResult r : in) {
-      res.add(toChangeInfo(out, changes, reviewed));
+      List<ChangeInfo> infos = toChangeInfo(out, r.changes(), reviewed);
+      if (r.moreChanges()) {
+        infos.get(infos.size() - 1)._moreChanges = true;
+      }
+      res.add(infos);
     }
     accountLoader.fill();
     return res;

gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java

@@ -14,9 +14,8 @@
 
 package com.google.gerrit.server.index;
 
-import static com.google.gerrit.common.data.GlobalCapability.DEFAULT_MAX_QUERY_LIMIT;
+import static com.google.common.base.Preconditions.checkArgument;
 
-import com.google.common.base.MoreObjects;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.gerrit.reviewdb.client.Change;
@@ -29,9 +28,9 @@ import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.AndSource;
 import com.google.gerrit.server.query.change.BasicChangeRewrites;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder;
 import com.google.gerrit.server.query.change.ChangeQueryRewriter;
 import com.google.gerrit.server.query.change.ChangeStatusPredicate;
+import com.google.gerrit.server.query.change.LimitPredicate;
 import com.google.gerrit.server.query.change.OrSource;
 import com.google.inject.Inject;
 
@@ -129,16 +128,14 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
   }
 
   @Override
-  public Predicate<ChangeData> rewrite(Predicate<ChangeData> in, int start)
+  public Predicate<ChangeData> rewrite(Predicate<ChangeData> in, int start,
-      throws QueryParseException {
+      int limit) throws QueryParseException {
+    checkArgument(limit > 0, "limit must be positive: %s", limit);
     ChangeIndex index = indexes.getSearchIndex();
     in = basicRewrites.rewrite(in);
-    int limit = MoreObjects.firstNonNull(
-        ChangeQueryBuilder.getLimit(in), DEFAULT_MAX_QUERY_LIMIT);
     // Increase the limit rather than skipping, since we don't know how many
     // skipped results would have been filtered out by the enclosing AndSource.
     limit += start;
-    limit = Math.max(limit, 1);
 
     Predicate<ChangeData> out = rewriteImpl(in, index, limit);
     if (in == out || out instanceof IndexPredicate) {
@@ -168,6 +165,9 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
       ChangeIndex index, int limit) throws QueryParseException {
     if (isIndexPredicate(in, index)) {
       return in;
+    } else if (in instanceof LimitPredicate) {
+      // Replace any limits with the limit provided by the caller.
+      return new LimitPredicate(limit);
     } else if (!isRewritePossible(in)) {
       return null; // magic to indicate "in" cannot be rewritten
     }

gerrit-server/src/main/java/com/google/gerrit/server/query/QueryBuilder.java

@@ -138,7 +138,8 @@ public abstract class QueryBuilder<T> {
    * @param p the predicate to find.
    * @param clazz type of the predicate instance.
    * @param name name of the operator.
-   * @return the predicate, null if not found.
+   * @return the first instance of a predicate having the given type, as found
+   *     by a depth-first search.
    */
   @SuppressWarnings("unchecked")
   public static <T, P extends OperatorPredicate<T>> P find(Predicate<T> p,

gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java

@@ -16,13 +16,11 @@ package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.query.IntPredicate;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryRewriter;
 import com.google.inject.Inject;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
-import com.google.inject.name.Named;
 
 public class BasicChangeRewrites extends QueryRewriter<ChangeData> {
   private static final ChangeQueryBuilder BUILDER = new ChangeQueryBuilder(
@@ -69,14 +67,6 @@ public class BasicChangeRewrites extends QueryRewriter<ChangeData> {
         ChangeStatusPredicate.forStatus(Change.Status.MERGED));
   }
 
-  @NoCostComputation
-  @Rewrite("A=(limit:*) B=(limit:*)")
-  public Predicate<ChangeData> r00_smallestLimit(
-      @Named("A") IntPredicate<ChangeData> a,
-      @Named("B") IntPredicate<ChangeData> b) {
-    return a.intValue() <= b.intValue() ? a : b;
-  }
-
   private static final class InvalidProvider<T> implements Provider<T> {
     @Override
     public T get() {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -45,7 +45,6 @@ import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ListChildProjects;
 import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.query.IntPredicate;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryBuilder;
 import com.google.gerrit.server.query.QueryParseException;
@@ -121,12 +120,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
   private static final QueryBuilder.Definition<ChangeData, ChangeQueryBuilder> mydef =
       new QueryBuilder.Definition<>(ChangeQueryBuilder.class);
 
-  @SuppressWarnings("unchecked")
-  public static Integer getLimit(Predicate<ChangeData> p) {
-    IntPredicate<?> ip = find(p, IntPredicate.class, FIELD_LIMIT);
-    return ip != null ? ip.intValue() : null;
-  }
-
   @VisibleForTesting
   public static class Arguments {
     final Provider<ReviewDb> db;
@@ -633,28 +626,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
   }
 
   @Operator
-  public Predicate<ChangeData> limit(String limit) {
+  public Predicate<ChangeData> limit(String limit) throws QueryParseException {
-    return limit(Integer.parseInt(limit));
+    return new LimitPredicate(Integer.parseInt(limit));
-  }
-
-  static class LimitPredicate extends IntPredicate<ChangeData> {
-    LimitPredicate(int limit) {
-      super(FIELD_LIMIT, limit);
-    }
-
-    @Override
-    public boolean match(ChangeData object) {
-      return true;
-    }
-
-    @Override
-    public int getCost() {
-      return 0;
-    }
-  }
-
-  public Predicate<ChangeData> limit(int limit) {
-    return new LimitPredicate(limit);
   }
 
   @Operator

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java

@@ -18,6 +18,6 @@ import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 
 public interface ChangeQueryRewriter {
-  Predicate<ChangeData> rewrite(Predicate<ChangeData> in, int start)
+  Predicate<ChangeData> rewrite(Predicate<ChangeData> in, int start, int limit)
       throws QueryParseException;
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/LimitPredicate.java

@@ -0,0 +1,47 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.change;
+
+import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_LIMIT;
+
+import com.google.gerrit.server.query.IntPredicate;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.QueryBuilder;
+import com.google.gerrit.server.query.QueryParseException;
+
+public class LimitPredicate extends IntPredicate<ChangeData> {
+  @SuppressWarnings("unchecked")
+  public static Integer getLimit(Predicate<ChangeData> p) {
+    IntPredicate<?> ip = QueryBuilder.find(p, IntPredicate.class, FIELD_LIMIT);
+    return ip != null ? ip.intValue() : null;
+  }
+
+  public LimitPredicate(int limit) throws QueryParseException {
+    super(ChangeQueryBuilder.FIELD_LIMIT, limit);
+    if (limit <= 0) {
+      throw new QueryParseException("limit must be positive: " + limit);
+    }
+  }
+
+  @Override
+  public boolean match(ChangeData object) {
+    return true;
+  }
+
+  @Override
+  public int getCost() {
+    return 0;
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java

@@ -31,7 +31,6 @@ import com.google.inject.Provider;
 
 import org.kohsuke.args4j.Option;
 
-import java.util.BitSet;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
@@ -136,21 +135,12 @@ public class QueryChanges implements RestReadView<TopLevelResource> {
   private List<List<ChangeInfo>> query0() throws OrmException,
       QueryParseException {
     int cnt = queries.size();
-    BitSet more = new BitSet(cnt);
+    List<QueryResult> results = imp.queryChanges(queries);
-    List<List<ChangeData>> data = imp.queryChanges(queries);
+    List<List<ChangeInfo>> res = json.addOptions(options)
-    for (int n = 0; n < cnt; n++) {
+        .formatQueryResults(results);
-      List<ChangeData> changes = data.get(n);
-      if (imp.getLimit() > 0 && changes.size() > imp.getLimit()) {
-        changes = changes.subList(0, imp.getLimit());
-        data.set(n, changes);
-        more.set(n, true);
-      }
-    }
-
-    List<List<ChangeInfo>> res = json.addOptions(options).formatList2(data);
     for (int n = 0; n < cnt; n++) {
       List<ChangeInfo> info = res.get(n);
-      if (more.get(n) && !info.isEmpty()) {
+      if (results.get(n).moreChanges()) {
         info.get(info.size() - 1)._moreChanges = true;
       }
     }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java

@@ -14,9 +14,8 @@
 
 package com.google.gerrit.server.query.change;
 
-import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.LabelTypes;
@@ -71,10 +70,10 @@ public class QueryProcessor {
   private final ChangeQueryRewriter queryRewriter;
   private final TrackingFooters trackingFooters;
   private final CurrentUser user;
-  private final int maxLimit;
+  private final int permittedLimit;
 
   private OutputFormat outputFormat = OutputFormat.TEXT;
-  private int limit;
+  private int limitFromCaller;
   private int start;
   private boolean includePatchSets;
   private boolean includeCurrentPatchSet;
@@ -88,7 +87,6 @@ public class QueryProcessor {
 
   private OutputStream outputStream = DisabledOutputStream.INSTANCE;
   private PrintWriter out;
-  private boolean moreResults;
 
   @Inject
   QueryProcessor(EventFactory eventFactory,
@@ -100,18 +98,13 @@ public class QueryProcessor {
     this.queryRewriter = queryRewriter;
     this.trackingFooters = trackingFooters;
     this.user = currentUser;
-    this.maxLimit = currentUser.getCapabilities()
+    this.permittedLimit = currentUser.getCapabilities()
       .getRange(GlobalCapability.QUERY_LIMIT)
       .getMax();
-    this.moreResults = false;
-  }
-
-  int getLimit() {
-    return limit;
   }
 
   void setLimit(int n) {
-    limit = n;
+    limitFromCaller = n;
   }
 
   public void setStart(int n) {
@@ -183,7 +176,7 @@ public class QueryProcessor {
    * there are more than {@code limit} matches and suggest to its own caller
    * that the query could be retried with {@link #setStart(int)}.
    */
-  public List<ChangeData> queryChanges(String queryString)
+  public QueryResult queryChanges(String queryString)
       throws OrmException, QueryParseException {
     return queryChanges(ImmutableList.of(queryString)).get(0);
   }
@@ -196,48 +189,52 @@ public class QueryProcessor {
    * there are more than {@code limit} matches and suggest to its own caller
    * that the query could be retried with {@link #setStart(int)}.
    */
-  public List<List<ChangeData>> queryChanges(List<String> queries)
+  public List<QueryResult> queryChanges(List<String> queries)
       throws OrmException, QueryParseException {
     final Predicate<ChangeData> visibleToMe = queryBuilder.is_visible();
     int cnt = queries.size();
 
     // Parse and rewrite all queries.
-    List<Integer> limits = Lists.newArrayListWithCapacity(cnt);
+    List<Integer> limits = new ArrayList<>(cnt);
-    List<ChangeDataSource> sources = Lists.newArrayListWithCapacity(cnt);
+    List<Predicate<ChangeData>> predicates = new ArrayList<>(cnt);
+    List<ChangeDataSource> sources = new ArrayList<>(cnt);
     for (String query : queries) {
       Predicate<ChangeData> q = parseQuery(query, visibleToMe);
-      Predicate<ChangeData> s = queryRewriter.rewrite(q, start);
+      int limit = getEffectiveLimit(q);
+      limits.add(limit);
+
+      // Always bump limit by 1, even if this results in exceeding the permitted
+      // max for this user. The only way to see if there are more changes is to
+      // ask for one more result from the query.
+      Predicate<ChangeData> s = queryRewriter.rewrite(q, start, limit + 1);
       if (!(s instanceof ChangeDataSource)) {
         q = Predicate.and(queryBuilder.status_open(), q);
-        s = queryRewriter.rewrite(q, start);
+        s = queryRewriter.rewrite(q, start, limit);
       }
       if (!(s instanceof ChangeDataSource)) {
         throw new QueryParseException("invalid query: " + s);
       }
+      predicates.add(s);
 
       // Don't trust QueryRewriter to have left the visible predicate.
+      // TODO(dborowitz): Probably we can.
       AndSource a = new AndSource(ImmutableList.of(s, visibleToMe), start);
-      limits.add(limit(q));
       sources.add(a);
     }
 
     // Run each query asynchronously, if supported.
-    List<ResultSet<ChangeData>> matches = Lists.newArrayListWithCapacity(cnt);
+    List<ResultSet<ChangeData>> matches = new ArrayList<>(cnt);
     for (ChangeDataSource s : sources) {
       matches.add(s.read());
     }
 
-    List<List<ChangeData>> out = Lists.newArrayListWithCapacity(cnt);
+    List<QueryResult> out = new ArrayList<>(cnt);
     for (int i = 0; i < cnt; i++) {
-      List<ChangeData> results = matches.get(i).toList();
+      out.add(QueryResult.create(
-      if (results.size() > maxLimit) {
+          queries.get(i),
-        moreResults = true;
+          predicates.get(i),
-      }
+          limits.get(i),
-      int limit = limits.get(i);
+          matches.get(i).toList()));
-      if (limit < results.size()) {
-        results = results.subList(0, limit);
-      }
-      out.add(results);
     }
     return out;
   }
@@ -258,9 +255,9 @@ public class QueryProcessor {
         final QueryStatsAttribute stats = new QueryStatsAttribute();
         stats.runTimeMilliseconds = TimeUtil.nowMs();
 
-        List<ChangeData> results = queryChanges(queryString);
+        QueryResult results = queryChanges(queryString);
         ChangeAttribute c = null;
-        for (ChangeData d : results) {
+        for (ChangeData d : results.changes()) {
           ChangeControl cc = d.changeControl().forUser(user);
 
           LabelTypes labelTypes = cc.getLabelTypes();
@@ -329,8 +326,8 @@ public class QueryProcessor {
           show(c);
         }
 
-        stats.rowCount = results.size();
+        stats.rowCount = results.changes().size();
-        if (moreResults) {
+        if (results.moreChanges()) {
           stats.resumeSortKey = c.sortKey;
         }
         stats.runTimeMilliseconds =
@@ -363,19 +360,25 @@ public class QueryProcessor {
   }
 
   boolean isDisabled() {
-    return maxLimit <= 0;
+    return permittedLimit <= 0;
   }
 
-  private int limit(Predicate<ChangeData> s) {
+  private int getEffectiveLimit(Predicate<ChangeData> p) {
-    int n = MoreObjects.firstNonNull(ChangeQueryBuilder.getLimit(s), maxLimit);
+    List<Integer> possibleLimits = new ArrayList<>(3);
-    return limit > 0 ? Math.min(n, limit) + 1 : n + 1;
+    possibleLimits.add(permittedLimit);
+    if (limitFromCaller > 0) {
+      possibleLimits.add(limitFromCaller);
+    }
+    Integer limitFromPredicate = LimitPredicate.getLimit(p);
+    if (limitFromPredicate != null) {
+      possibleLimits.add(limitFromPredicate);
+    }
+    return Ordering.natural().min(possibleLimits);
   }
 
   private Predicate<ChangeData> parseQuery(String queryString,
-      final Predicate<ChangeData> visibleToMe) throws QueryParseException {
+      Predicate<ChangeData> visibleToMe) throws QueryParseException {
-    return Predicate.and(queryBuilder.parse(queryString),
+    return Predicate.and(queryBuilder.parse(queryString), visibleToMe);
-        queryBuilder.limit(limit > 0 ? Math.min(limit, maxLimit) + 1 : maxLimit),
-        visibleToMe);
   }
 
   private void show(Object data) {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryResult.java

@@ -0,0 +1,55 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.change;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.server.query.Predicate;
+
+import java.util.List;
+
+/** Results of a query over changes. */
+@AutoValue
+public abstract class QueryResult {
+  static QueryResult create(String query, Predicate<ChangeData> predicate,
+      int limit, List<ChangeData> changes) {
+    boolean moreChanges;
+    if (changes.size() > limit) {
+      moreChanges = true;
+      changes = changes.subList(0, limit);
+    } else {
+      moreChanges = false;
+    }
+    return new AutoValue_QueryResult(query, predicate, changes, moreChanges);
+  }
+
+  /** @return the original query string. */
+  public abstract String query();
+
+  /**
+   * @return the predicate after all rewriting and other modification by the
+   *     query subsystem.
+   */
+  public abstract Predicate<ChangeData> predicate();
+
+  /** @return the query results. */
+  public abstract List<ChangeData> changes();
+
+  /**
+   * @return whether the query could be retried with
+   *     {@link QueryProcessor#setStart(int)} to produce more results. Never
+   *     true if {@link #changes()} is empty.
+   */
+  public abstract boolean moreChanges();
+}

gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java

@@ -97,7 +97,7 @@ public class IndexRewriteTest {
         parse("-status:abandoned (status:open OR status:merged)");
     assertEquals(
         query(parse("status:new OR status:submitted OR status:draft OR status:merged")),
-        rewrite.rewrite(in, 0));
+        rewrite.rewrite(in, 0, DEFAULT_MAX_QUERY_LIMIT));
   }
 
   @Test
@@ -158,24 +158,26 @@ public class IndexRewriteTest {
   }
 
   @Test
-  public void testLimit() throws Exception {
+  public void testLimitArgumentOverridesAllLimitPredicates() throws Exception {
-    Predicate<ChangeData> in = parse("file:a limit:3");
+    Predicate<ChangeData> in = parse("limit:1 file:a limit:3");
-    Predicate<ChangeData> out = rewrite(in);
+    Predicate<ChangeData> out = rewrite(in, 5);
     assertSame(AndSource.class, out.getClass());
     assertEquals(ImmutableList.of(
-          query(in.getChild(0), 3),
+          query(in.getChild(1), 5),
-          in.getChild(1)),
+          parse("limit:5"),
+          parse("limit:5")),
         out.getChildren());
   }
 
   @Test
   public void testStartIncreasesLimit() throws Exception {
+    int n = 3;
     Predicate<ChangeData> f = parse("file:a");
-    Predicate<ChangeData> l = parse("limit:3");
+    Predicate<ChangeData> l = parse("limit:" + n);
     Predicate<ChangeData> in = and(f, l);
-    assertEquals(and(query(f, 3), l), rewrite.rewrite(in, 0));
+    assertEquals(and(query(f, 3), parse("limit:3")), rewrite.rewrite(in, 0, n));
-    assertEquals(and(query(f, 4), l), rewrite.rewrite(in, 1));
+    assertEquals(and(query(f, 4), parse("limit:4")), rewrite.rewrite(in, 1, n));
-    assertEquals(and(query(f, 5), l), rewrite.rewrite(in, 2));
+    assertEquals(and(query(f, 5), parse("limit:5")), rewrite.rewrite(in, 2, n));
   }
 
   @Test
@@ -220,7 +222,12 @@ public class IndexRewriteTest {
 
   private Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
       throws QueryParseException {
-    return rewrite.rewrite(in, 0);
+    return rewrite.rewrite(in, 0, DEFAULT_MAX_QUERY_LIMIT);
+  }
+
+  private Predicate<ChangeData> rewrite(Predicate<ChangeData> in, int limit)
+      throws QueryParseException {
+    return rewrite.rewrite(in, 0, limit);
   }
 
   private IndexedChangeQuery query(Predicate<ChangeData> p)

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -543,9 +543,22 @@ public abstract class AbstractQueryChangesTest {
 
     List<ChangeInfo> results;
     for (int i = 1; i <= n + 2; i++) {
+      int expectedSize;
+      Boolean expectedMoreChanges;
+      if (i < n) {
+        expectedSize = i;
+        expectedMoreChanges = true;
+      } else {
+        expectedSize = n;
+        expectedMoreChanges = null;
+      }
       results = query("status:new limit:" + i);
-      assertThat(results).hasSize(Math.min(i, n));
+      String msg = "i=" + i;
+      assert_().withFailureMessage(msg).that(results).hasSize(expectedSize);
       assertResultEquals(last, results.get(0));
+      assert_().withFailureMessage(msg)
+          .that(results.get(results.size() - 1)._moreChanges)
+          .isEqualTo(expectedMoreChanges);
     }
   }