Fix queries with multiple non-index change datasource predicates
Currently, a query like "A OR B" where both A and B are non-index change
datasource predicates, is re-written like:
(status:new OR status:merged OR status:abandoned)
AND
(A OR B)
This makes the query really slow and can even result in failure if the
number of total changes is very large.
Change 117790 fixed similar issue for queries where A and B are
subexpressions that can be answered by the index but are re-written
internally, extend the solution for current use-case where A and B are
non-index change datasource predicates.
Change-Id: If070e8e8fd5f9338a86803654e2b1ade87c4285b
Release-Notes: queries with multiple non-index datasource predicates are re-written correctly
(cherry picked from commit 8486e7b2e340563a18edcbc639379f0dc7fd0e95)
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
index bb4b24c..b022f43 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
@@ -221,6 +221,9 @@
isIndexed.set(i);
newChildren.add(c);
} else if (nc == null /* cannot rewrite c */) {
+ if (c instanceof ChangeDataSource) {
+ changeSource.set(i);
+ }
notIndexed.set(i);
newChildren.add(c);
} else {
@@ -235,6 +238,9 @@
if (isIndexed.cardinality() == n) {
return in; // All children are indexed, leave as-is for parent.
} else if (notIndexed.cardinality() == n) {
+ if (changeSource.cardinality() == n) {
+ return copy(in, newChildren);
+ }
return null; // Can't rewrite any children, so cannot rewrite in.
} else if (rewritten.cardinality() == n) {
// All children were rewritten.
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index 26e9e54..19f39a7 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeStatusPredicate;
+import com.google.gerrit.server.query.change.OrSource;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.Set;
@@ -106,6 +107,22 @@
}
@Test
+ public void nonIndexOrSourcePredicates() throws Exception {
+ Predicate<ChangeData> in = parse("baz:a OR baz:b");
+ Predicate<ChangeData> out = rewrite(in);
+ assertThat(out.getClass()).isSameInstanceAs(OrSource.class);
+ assertThat(out.getChildren()).containsExactly(parse("baz:a"), parse("baz:b")).inOrder();
+ }
+
+ @Test
+ public void nonIndexAndSourcePredicates() throws Exception {
+ Predicate<ChangeData> in = parse("baz:a baz:b");
+ Predicate<ChangeData> out = rewrite(in);
+ assertThat(out.getClass()).isSameInstanceAs(AndChangeSource.class);
+ assertThat(out.getChildren()).containsExactly(parse("baz:a"), parse("baz:b")).inOrder();
+ }
+
+ @Test
public void oneIndexPredicate() throws Exception {
Predicate<ChangeData> in = parse("foo:a file:b");
Predicate<ChangeData> out = rewrite(in);
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index e879170..13badb5 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -14,16 +14,48 @@
package com.google.gerrit.server.index.change;
+import com.google.gerrit.index.query.FieldBundle;
import com.google.gerrit.index.query.OperatorPredicate;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryBuilder;
+import com.google.gerrit.index.query.ResultSet;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeDataSource;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import org.eclipse.jgit.lib.Config;
import org.junit.Ignore;
@Ignore
public class FakeQueryBuilder extends ChangeQueryBuilder {
+ public static class FakeNonIndexSourcePredicate extends OperatorPredicate<ChangeData>
+ implements ChangeDataSource {
+ private static final String operator = "baz";
+
+ public FakeNonIndexSourcePredicate(String value) {
+ super(operator, value);
+ }
+
+ @Override
+ public ResultSet<ChangeData> read() {
+ return null;
+ }
+
+ @Override
+ public ResultSet<FieldBundle> readRaw() {
+ return null;
+ }
+
+ @Override
+ public int getCardinality() {
+ return 0;
+ }
+
+ @Override
+ public boolean hasChange() {
+ return false;
+ }
+ }
+
FakeQueryBuilder(ChangeIndexCollection indexes) {
super(
new QueryBuilder.Definition<>(FakeQueryBuilder.class),
@@ -62,6 +94,11 @@
}
@Operator
+ public Predicate<ChangeData> baz(String value) {
+ return new FakeNonIndexSourcePredicate(value);
+ }
+
+ @Operator
public Predicate<ChangeData> foo(String value) {
return predicate("foo", value);
}
