Only rewrite IndexPredicates for fields in the current schema

Change-Id: I6fc584fdc59c31b21b495c6c66f6ac0a98b32301

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

@@ -135,9 +135,10 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
   public Predicate<ChangeData> rewrite(Predicate<ChangeData> in) {
     in = basicRewrites.rewrite(in);
 
-    Predicate<ChangeData> out = rewriteImpl(in);
+    ChangeIndex index = indexes.getSearchIndex();
+    Predicate<ChangeData> out = rewriteImpl(in, index);
     if (in == out || out instanceof IndexPredicate) {
-      return query(out);
+      return query(out, index);
     } else if (out == null /* cannot rewrite */) {
       return in;
     } else {
@@ -149,14 +150,16 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
    * Rewrite a single predicate subtree.
    *
    * @param in predicate to rewrite.
+   * @param index index whose schema determines which fields are indexed.
    * @return {@code null} if no part of this subtree can be queried in the
    *     index directly. {@code in} if this subtree and all its children can be
    *     queried directly in the index. Otherwise, a predicate that is
    *     semantically equivalent, with some of its subtrees wrapped to query the
    *     index directly.
    */
-  private Predicate<ChangeData> rewriteImpl(Predicate<ChangeData> in) {
-    if (in instanceof IndexPredicate) {
+  private Predicate<ChangeData> rewriteImpl(Predicate<ChangeData> in,
+      ChangeIndex index) {
+    if (isIndexPredicate(in, index)) {
       return in;
     } else if (!isRewritePossible(in)) {
       return null; // magic to indicate "in" cannot be rewritten
@@ -169,7 +172,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
     List<Predicate<ChangeData>> newChildren = Lists.newArrayListWithCapacity(n);
     for (int i = 0; i < n; i++) {
       Predicate<ChangeData> c = in.getChild(i);
-      Predicate<ChangeData> nc = rewriteImpl(c);
+      Predicate<ChangeData> nc = rewriteImpl(c, index);
       if (nc == c) {
         isIndexed.set(i);
         newChildren.add(c);
@@ -189,16 +192,25 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
     } else if (rewritten.cardinality() == n) {
       return in.copy(newChildren); // All children were rewritten.
     }
-    return partitionChildren(in, newChildren, isIndexed);
+    return partitionChildren(in, newChildren, isIndexed, index);
+  }
+
+  private boolean isIndexPredicate(Predicate<ChangeData> in, ChangeIndex index) {
+    if (!(in instanceof IndexPredicate)) {
+      return false;
+    }
+    IndexPredicate<ChangeData> p = (IndexPredicate<ChangeData>) in;
+    return index.getSchema().getFields().containsKey(p.getField().getName());
   }
 
   private Predicate<ChangeData> partitionChildren(
       Predicate<ChangeData> in,
       List<Predicate<ChangeData>> newChildren,
-      BitSet isIndexed) {
+      BitSet isIndexed,
+      ChangeIndex index) {
     if (isIndexed.cardinality() == 1) {
       int i = isIndexed.nextSetBit(0);
-      newChildren.add(0, query(newChildren.remove(i)));
+      newChildren.add(0, query(newChildren.remove(i), index));
       return copy(in, newChildren);
     }
 
@@ -218,7 +230,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
         all.add(c);
       }
     }
-    all.add(0, query(in.copy(indexed)));
+    all.add(0, query(in.copy(indexed), index));
     return copy(in, all);
   }
 
@@ -233,9 +245,9 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
     return in.copy(all);
   }
 
-  private IndexedChangeQuery query(Predicate<ChangeData> p) {
+  private IndexedChangeQuery query(Predicate<ChangeData> p, ChangeIndex index) {
     try {
-      return new IndexedChangeQuery(indexes.getSearchIndex(), p);
+      return new IndexedChangeQuery(index, p);
     } catch (QueryParseException e) {
       throw new IllegalStateException(
           "Failed to convert " + p + " to index predicate", e);

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.index;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
 
 /** Specific version of a secondary index schema. */
@@ -23,6 +24,13 @@ public class Schema<T> {
   private int version;
 
   protected Schema(boolean release, Iterable<FieldDef<T, ?>> fields) {
+    this(0, release, fields);
+  }
+
+  @VisibleForTesting
+  public Schema(int version, boolean release,
+      Iterable<FieldDef<T, ?>> fields) {
+    this.version = version;
     this.release = release;
     ImmutableMap.Builder<String, FieldDef<T, ?>> b = ImmutableMap.builder();
     for (FieldDef<T, ?> f : fields) {

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

@@ -23,6 +23,7 @@ import static com.google.gerrit.reviewdb.client.Change.Status.SUBMITTED;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.query.AndPredicate;
 import com.google.gerrit.server.query.OperatorPredicate;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
@@ -41,7 +42,22 @@ import java.util.Set;
 
 @SuppressWarnings("unchecked")
 public class IndexRewriteTest extends TestCase {
+  private static Schema<ChangeData> V1 = new Schema<ChangeData>(
+      1, false, ImmutableList.<FieldDef<ChangeData, ?>> of(
+          ChangeField.STATUS));
+
+  private static Schema<ChangeData> V2 = new Schema<ChangeData>(
+      2, false, ImmutableList.of(
+          ChangeField.STATUS,
+          ChangeField.FILE));
+
   private static class DummyIndex implements ChangeIndex {
+    private final Schema<ChangeData> schema;
+
+    private DummyIndex(Schema<ChangeData> schema) {
+      this.schema = schema;
+    }
+
     @Override
     public ListenableFuture<Void> insert(ChangeData cd) {
       throw new UnsupportedOperationException();
@@ -70,7 +86,7 @@ public class IndexRewriteTest extends TestCase {
 
     @Override
     public Schema<ChangeData> getSchema() {
-      throw new UnsupportedOperationException();
+      return schema;
     }
 
     @Override
@@ -150,7 +166,7 @@ public class IndexRewriteTest extends TestCase {
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    index = new DummyIndex();
+    index = new DummyIndex(V2);
     indexes = new IndexCollection();
     indexes.setSearchIndex(index);
     queryBuilder = new QueryBuilder();
@@ -265,6 +281,19 @@ public class IndexRewriteTest extends TestCase {
         status("(is:new is:draft) OR (is:merged OR is:submitted)"));
   }
 
+  public void testUnsupportedIndexOperator() throws Exception {
+    Predicate<ChangeData> in = parse("status:merged file:a");
+    assertEquals(query(in), rewrite(in));
+
+    indexes.setSearchIndex(new DummyIndex(V1));
+    Predicate<ChangeData> out = rewrite(in);
+    assertTrue(out instanceof AndPredicate);
+    assertEquals(ImmutableList.of(
+          query(in.getChild(0)),
+          in.getChild(1)),
+        out.getChildren());
+  }
+
   private Predicate<ChangeData> parse(String query) throws QueryParseException {
     return queryBuilder.parse(query);
   }