From 9c4720405b088785bb181df3d7bca8410b1501e3 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 26 Jun 2013 21:42:23 -0700 Subject: [PATCH] Don't run SQL rewrites when secondary index is enabled If the secondary index is enabled on a server the index handles all queries. There is no reason to run the SQL rewrites or to leave predicates in the predicate tree without being wrapped by the IndexedChangeQuery predicate. Change-Id: I18defed2a5a6003b9ae6ba227e29864b5c912e7b --- .../gerrit/lucene/LuceneChangeIndex.java | 11 +- .../google/gerrit/lucene/QueryBuilder.java | 4 +- .../server/config/GerritGlobalModule.java | 2 - .../gerrit/server/index/IndexModule.java | 6 +- .../gerrit/server/index/IndexPredicate.java | 8 - .../gerrit/server/index/IndexRewriteImpl.java | 261 +++++++ ...teWrapper.java => IndexedChangeQuery.java} | 29 +- .../gerrit/server/index/NoIndexModule.java | 5 +- .../gerrit/server/query/QueryRewriter.java | 27 +- .../gerrit/server/query/change/AndSource.java | 6 +- .../query/change/BasicChangeRewrites.java | 109 +++ .../query/change/ChangeQueryBuilder.java | 8 +- .../query/change/ChangeQueryRewriter.java | 699 +----------------- .../query/change/ChangeStatusPredicate.java | 6 +- .../server/query/change/CommentPredicate.java | 5 - .../query/change/EqualsFilePredicate.java | 5 - .../server/query/change/IndexRewrite.java | 37 - .../server/query/change/IndexRewriteImpl.java | 222 ------ .../server/query/change/MessagePredicate.java | 5 - .../gerrit/server/query/change/OrSource.java | 4 +- .../server/query/change/SqlRewriterImpl.java | 631 ++++++++++++++++ .../change => index}/IndexRewriteTest.java | 81 +- .../google/gerrit/solr/SolrChangeIndex.java | 11 +- 23 files changed, 1132 insertions(+), 1050 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java rename gerrit-server/src/main/java/com/google/gerrit/server/index/{PredicateWrapper.java => IndexedChangeQuery.java} (83%) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewrite.java delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java rename gerrit-server/src/test/java/com/google/gerrit/server/{query/change => index}/IndexRewriteTest.java (78%) diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 5e8cac5981..4f55675554 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -16,8 +16,8 @@ package com.google.gerrit.lucene; import static com.google.gerrit.lucene.IndexVersionCheck.SCHEMA_VERSIONS; import static com.google.gerrit.lucene.IndexVersionCheck.gerritIndexConfig; -import static com.google.gerrit.server.query.change.IndexRewriteImpl.CLOSED_STATUSES; -import static com.google.gerrit.server.query.change.IndexRewriteImpl.OPEN_STATUSES; +import static com.google.gerrit.server.index.IndexRewriteImpl.CLOSED_STATUSES; +import static com.google.gerrit.server.index.IndexRewriteImpl.OPEN_STATUSES; import com.google.common.base.Function; import com.google.common.collect.ImmutableSet; @@ -34,11 +34,11 @@ import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.FieldDef.FillArgs; import com.google.gerrit.server.index.FieldType; +import com.google.gerrit.server.index.IndexRewriteImpl; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeDataSource; -import com.google.gerrit.server.query.change.IndexRewriteImpl; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; @@ -258,6 +258,11 @@ public class LuceneChangeIndex implements ChangeIndex, LifecycleListener { return false; } + @Override + public String toString() { + return query.toString(); + } + @Override public ResultSet read() throws OrmException { IndexSearcher[] searchers = new IndexSearcher[indexes.size()]; diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java index e1c08d2f56..e1dc7c19ca 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java @@ -121,8 +121,8 @@ public class QueryBuilder { private static Query sortKeyQuery(SortKeyPredicate p) { return NumericRangeQuery.newLongRange( p.getField().getName(), - p.getMinValue(), - p.getMaxValue(), + p.getMinValue() != Long.MIN_VALUE ? p.getMinValue() : null, + p.getMaxValue() != Long.MAX_VALUE ? p.getMaxValue() : null, true, true); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 7b242d95ec..2e69ff0b25 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -106,7 +106,6 @@ import com.google.gerrit.server.project.ProjectNode; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.SectionSortCache; import com.google.gerrit.server.query.change.ChangeQueryBuilder; -import com.google.gerrit.server.query.change.ChangeQueryRewriter; import com.google.gerrit.server.ssh.SshAddressesModule; import com.google.gerrit.server.tools.ToolsCatalog; import com.google.gerrit.server.util.IdGenerator; @@ -172,7 +171,6 @@ public class GerritGlobalModule extends FactoryModule { install(ThreadLocalRequestContext.module()); bind(AccountResolver.class); - bind(ChangeQueryRewriter.class); factory(AccountInfoCacheFactory.Factory.class); factory(AddReviewerSender.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java index 5bdbd96a99..a04fe80282 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java @@ -19,8 +19,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.git.WorkQueue.Executor; -import com.google.gerrit.server.query.change.IndexRewrite; -import com.google.gerrit.server.query.change.IndexRewriteImpl; +import com.google.gerrit.server.query.change.ChangeQueryRewriter; import com.google.inject.AbstractModule; import com.google.inject.Injector; import com.google.inject.Key; @@ -56,7 +55,8 @@ public class IndexModule extends AbstractModule { @Override protected void configure() { bind(ChangeIndexer.class).to(ChangeIndexerImpl.class); - bind(IndexRewrite.class).to(IndexRewriteImpl.class); + bind(ChangeQueryRewriter.class).to(IndexRewriteImpl.class); + bind(IndexRewriteImpl.BasicRewritesImpl.class); } @Provides diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexPredicate.java index 82e3aeb261..d3b9e95a7c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexPredicate.java @@ -37,12 +37,4 @@ public abstract class IndexPredicate extends OperatorPredicate { public FieldType getType() { return def.getType(); } - - /** - * @return whether this predicate can only be satisfied by looking at the - * secondary index, i.e. it cannot be expressed as a query over the DB. - */ - public boolean isIndexOnly() { - return false; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java new file mode 100644 index 0000000000..45616effc2 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java @@ -0,0 +1,261 @@ +// Copyright (C) 2013 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.index; + +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Change.Status; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.query.AndPredicate; +import com.google.gerrit.server.query.NotPredicate; +import com.google.gerrit.server.query.OrPredicate; +import com.google.gerrit.server.query.Predicate; +import com.google.gerrit.server.query.QueryParseException; +import com.google.gerrit.server.query.QueryRewriter; +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.ChangeQueryRewriter; +import com.google.gerrit.server.query.change.ChangeStatusPredicate; +import com.google.gerrit.server.query.change.OrSource; +import com.google.gerrit.server.query.change.SqlRewriterImpl; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.util.BitSet; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; + +/** Rewriter that pushes boolean logic into the secondary index. */ +public class IndexRewriteImpl implements ChangeQueryRewriter { + /** Set of all open change statuses. */ + public static final Set OPEN_STATUSES; + + /** Set of all closed change statuses. */ + public static final Set CLOSED_STATUSES; + + static { + EnumSet open = EnumSet.noneOf(Change.Status.class); + EnumSet closed = EnumSet.noneOf(Change.Status.class); + for (Change.Status s : Change.Status.values()) { + if (s.isOpen()) { + open.add(s); + } else { + closed.add(s); + } + } + OPEN_STATUSES = Sets.immutableEnumSet(open); + CLOSED_STATUSES = Sets.immutableEnumSet(closed); + } + + /** + * Get the set of statuses that changes matching the given predicate may have. + * + * @param in predicate + * @return the maximal set of statuses that any changes matching the input + * predicates may have, based on examining boolean and + * {@link ChangeStatusPredicate}s. + */ + public static EnumSet getPossibleStatus(Predicate in) { + EnumSet s = extractStatus(in); + return s != null ? s : EnumSet.allOf(Change.Status.class); + } + + private static EnumSet extractStatus(Predicate in) { + if (in instanceof ChangeStatusPredicate) { + return EnumSet.of(((ChangeStatusPredicate) in).getStatus()); + } else if (in instanceof NotPredicate) { + EnumSet s = extractStatus(in.getChild(0)); + return s != null ? EnumSet.complementOf(s) : null; + } else if (in instanceof OrPredicate) { + EnumSet r = null; + int childrenWithStatus = 0; + for (int i = 0; i < in.getChildCount(); i++) { + EnumSet c = extractStatus(in.getChild(i)); + if (c != null) { + if (r == null) { + r = EnumSet.noneOf(Change.Status.class); + } + r.addAll(c); + childrenWithStatus++; + } + } + if (r != null && childrenWithStatus < in.getChildCount()) { + // At least one child supplied a status but another did not. + // Assume all statuses for the children that did not feed a + // status at this part of the tree. This matches behavior if + // the child was used at the root of a query. + return EnumSet.allOf(Change.Status.class); + } + return r; + } else if (in instanceof AndPredicate) { + EnumSet r = null; + for (int i = 0; i < in.getChildCount(); i++) { + EnumSet c = extractStatus(in.getChild(i)); + if (c != null) { + if (r == null) { + r = EnumSet.allOf(Change.Status.class); + } + r.retainAll(c); + } + } + return r; + } + return null; + } + + private final ChangeIndex index; + private final Provider db; + private final BasicRewritesImpl basicRewrites; + + @Inject + IndexRewriteImpl(ChangeIndex index, + Provider db, + BasicRewritesImpl basicRewrites) { + this.index = index; + this.db = db; + this.basicRewrites = basicRewrites; + } + + @Override + public Predicate rewrite(Predicate in) { + in = basicRewrites.rewrite(in); + + Predicate out = rewriteImpl(in); + if (in == out || out instanceof IndexPredicate) { + return query(out); + } else if (out == null /* cannot rewrite */) { + return in; + } else { + return out; + } + } + + /** + * Rewrite a single predicate subtree. + * + * @param in predicate to rewrite. + * @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 rewriteImpl(Predicate in) { + if (in instanceof IndexPredicate) { + return in; + } else if (!isRewritePossible(in)) { + return null; // magic to indicate "in" cannot be rewritten + } + + int n = in.getChildCount(); + BitSet isIndexed = new BitSet(n); + BitSet notIndexed = new BitSet(n); + BitSet rewritten = new BitSet(n); + List> newChildren = Lists.newArrayListWithCapacity(n); + for (int i = 0; i < n; i++) { + Predicate c = in.getChild(i); + Predicate nc = rewriteImpl(c); + if (nc == c) { + isIndexed.set(i); + newChildren.add(c); + } else if (nc == null /* cannot rewrite c */) { + notIndexed.set(i); + newChildren.add(c); + } else { + rewritten.set(i); + newChildren.add(nc); + } + } + + if (isIndexed.cardinality() == n) { + return in; // All children are indexed, leave as-is for parent. + } else if (notIndexed.cardinality() == n) { + return null; // Can't rewrite any children, so cannot rewrite in. + } else if (rewritten.cardinality() == n) { + return in.copy(newChildren); // All children were rewritten. + } + return partitionChildren(in, newChildren, isIndexed); + } + + private Predicate partitionChildren( + Predicate in, + List> newChildren, + BitSet isIndexed) { + if (isIndexed.cardinality() == 1) { + int i = isIndexed.nextSetBit(0); + newChildren.add(0, query(newChildren.remove(i))); + return copy(in, newChildren); + } + + // Group all indexed predicates into a wrapped subtree. + List> indexed = + Lists.newArrayListWithCapacity(isIndexed.cardinality()); + + List> all = + Lists.newArrayListWithCapacity( + newChildren.size() - isIndexed.cardinality() + 1); + + for (int i = 0; i < newChildren.size(); i++) { + Predicate c = newChildren.get(i); + if (isIndexed.get(i)) { + indexed.add(c); + } else { + all.add(c); + } + } + all.add(0, query(in.copy(indexed))); + return copy(in, all); + } + + private Predicate copy( + Predicate in, + List> all) { + if (in instanceof AndPredicate) { + return new AndSource(db, all); + } else if (in instanceof OrPredicate) { + return new OrSource(all); + } + return in.copy(all); + } + + private IndexedChangeQuery query(Predicate p) { + try { + return new IndexedChangeQuery(index, p); + } catch (QueryParseException e) { + throw new IllegalStateException( + "Failed to convert " + p + " to index predicate", e); + } + } + + private static boolean isRewritePossible(Predicate p) { + return p.getChildCount() > 0 && ( + p instanceof AndPredicate + || p instanceof OrPredicate + || p instanceof NotPredicate); + } + + static class BasicRewritesImpl extends BasicChangeRewrites { + private static final QueryRewriter.Definition mydef = + new QueryRewriter.Definition( + BasicRewritesImpl.class, SqlRewriterImpl.BUILDER); + @Inject + BasicRewritesImpl(Provider db) { + super(mydef, db); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java similarity index 83% rename from gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java rename to gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java index 39eed81b9d..175208abb0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.index; import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; @@ -35,17 +36,35 @@ import java.util.List; * the secondary index; such predicates must also implement * {@link ChangeDataSource} to be chosen by the query processor. */ -public class PredicateWrapper extends Predicate implements - ChangeDataSource { +public class IndexedChangeQuery extends Predicate + implements ChangeDataSource { private final Predicate pred; private final ChangeDataSource source; - public PredicateWrapper(ChangeIndex index, Predicate pred) + public IndexedChangeQuery(ChangeIndex index, Predicate pred) throws QueryParseException { this.pred = pred; this.source = index.getSource(pred); } + @Override + public int getChildCount() { + return 1; + } + + @Override + public Predicate getChild(int i) { + if (i == 0) { + return pred; + } + throw new ArrayIndexOutOfBoundsException(i); + } + + @Override + public List> getChildren() { + return ImmutableList.of(pred); + } + @Override public int getCardinality() { return source.getCardinality(); @@ -115,11 +134,11 @@ public class PredicateWrapper extends Predicate implements public boolean equals(Object other) { return other != null && getClass() == other.getClass() - && pred.equals(((PredicateWrapper) other).pred); + && pred.equals(((IndexedChangeQuery) other).pred); } @Override public String toString() { - return "index(" + pred + ")"; + return "index(" + source + ")"; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java index 341753462d..8c552d8e84 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java @@ -14,7 +14,8 @@ package com.google.gerrit.server.index; -import com.google.gerrit.server.query.change.IndexRewrite; +import com.google.gerrit.server.query.change.ChangeQueryRewriter; +import com.google.gerrit.server.query.change.SqlRewriterImpl; import com.google.inject.AbstractModule; public class NoIndexModule extends AbstractModule { @@ -26,6 +27,6 @@ public class NoIndexModule extends AbstractModule { protected void configure() { bind(ChangeIndex.class).toInstance(ChangeIndex.DISABLED); bind(ChangeIndexer.class).toInstance(ChangeIndexer.DISABLED); - bind(IndexRewrite.class).toInstance(IndexRewrite.DISABLED); + bind(ChangeQueryRewriter.class).to(SqlRewriterImpl.class); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryRewriter.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryRewriter.java index b6a08d32e6..6088d8e3a8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryRewriter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryRewriter.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.query; +import com.google.common.collect.Lists; import com.google.inject.name.Named; import java.lang.annotation.Annotation; @@ -27,7 +28,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Comparator; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; @@ -65,19 +66,13 @@ public abstract class QueryRewriter { private final List> rewriteRules; public Definition(Class clazz, QueryBuilder qb) { - rewriteRules = new ArrayList>(); + rewriteRules = Lists.newArrayList(); Class c = clazz; while (c != QueryRewriter.class) { - final Method[] declared = c.getDeclaredMethods(); - Arrays.sort(declared, new Comparator() { - @Override - public int compare(Method o1, Method o2) { - return o1.getName().compareTo(o2.getName()); - } - }); + Method[] declared = c.getDeclaredMethods(); for (Method m : declared) { - final Rewrite rp = m.getAnnotation(Rewrite.class); + Rewrite rp = m.getAnnotation(Rewrite.class); if ((m.getModifiers() & Modifier.ABSTRACT) != Modifier.ABSTRACT && (m.getModifiers() & Modifier.PUBLIC) == Modifier.PUBLIC && rp != null) { @@ -86,6 +81,7 @@ public abstract class QueryRewriter { } c = c.getSuperclass(); } + Collections.sort(rewriteRules); } } @@ -340,7 +336,7 @@ public abstract class QueryRewriter { } /** Applies a rewrite rule to a Predicate. */ - protected interface RewriteRule { + protected interface RewriteRule extends Comparable> { /** * Apply a rewrite rule to the Predicate. * @@ -463,6 +459,15 @@ public abstract class QueryRewriter { final String msg = "Cannot apply " + method.getName(); return new IllegalArgumentException(msg, e); } + + @Override + public int compareTo(RewriteRule in) { + if (in instanceof MethodRewrite) { + return method.getName().compareTo( + ((MethodRewrite) in).method.getName()); + } + return 1; + } } private static Predicate removeDuplicates(Predicate in) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AndSource.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AndSource.java index 7f726a7cd0..0555fc758f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AndSource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AndSource.java @@ -34,7 +34,8 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; -class AndSource extends AndPredicate implements ChangeDataSource { +public class AndSource extends AndPredicate + implements ChangeDataSource { private static final Comparator> CMP = new Comparator>() { @Override @@ -75,7 +76,8 @@ class AndSource extends AndPredicate implements ChangeDataSource { private final Provider db; private int cardinality = -1; - AndSource(Provider db, Collection> that) { + public AndSource(Provider db, + Collection> that) { super(sort(that)); this.db = db; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java new file mode 100644 index 0000000000..1d2c9d2766 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java @@ -0,0 +1,109 @@ +// Copyright (C) 2013 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.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +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.OutOfScopeException; +import com.google.inject.Provider; +import com.google.inject.name.Named; + +public abstract class BasicChangeRewrites extends QueryRewriter { + protected static final ChangeQueryBuilder BUILDER = new ChangeQueryBuilder( + new ChangeQueryBuilder.Arguments( // + new InvalidProvider(), // + new InvalidProvider(), // + null, null, null, null, null, // + null, null, null, null, null), null); + + protected final Provider dbProvider; + + protected BasicChangeRewrites( + Definition> def, + Provider dbProvider) { + super(def); + this.dbProvider = dbProvider; + } + + @Rewrite("-status:open") + @NoCostComputation + public Predicate r00_notOpen() { + return ChangeStatusPredicate.closed(dbProvider); + } + + @Rewrite("-status:closed") + @NoCostComputation + public Predicate r00_notClosed() { + return ChangeStatusPredicate.open(dbProvider); + } + + @SuppressWarnings("unchecked") + @NoCostComputation + @Rewrite("-status:merged") + public Predicate r00_notMerged() { + return or(ChangeStatusPredicate.open(dbProvider), + new ChangeStatusPredicate(dbProvider, Change.Status.ABANDONED)); + } + + @SuppressWarnings("unchecked") + @NoCostComputation + @Rewrite("-status:abandoned") + public Predicate r00_notAbandoned() { + return or(ChangeStatusPredicate.open(dbProvider), + new ChangeStatusPredicate(dbProvider, Change.Status.MERGED)); + } + + @SuppressWarnings("unchecked") + @NoCostComputation + @Rewrite("sortkey_before:z A=(age:*)") + public Predicate r00_ageToSortKey(@Named("A") AgePredicate a) { + String cut = ChangeUtil.sortKey(a.getCut(), Integer.MAX_VALUE); + return and(new SortKeyPredicate.Before(dbProvider, cut), a); + } + + @NoCostComputation + @Rewrite("A=(limit:*) B=(limit:*)") + public Predicate r00_smallestLimit( + @Named("A") IntPredicate a, + @Named("B") IntPredicate b) { + return a.intValue() <= b.intValue() ? a : b; + } + + @NoCostComputation + @Rewrite("A=(sortkey_before:*) B=(sortkey_before:*)") + public Predicate r00_oldestSortKey( + @Named("A") SortKeyPredicate.Before a, + @Named("B") SortKeyPredicate.Before b) { + return a.getValue().compareTo(b.getValue()) <= 0 ? a : b; + } + + @NoCostComputation + @Rewrite("A=(sortkey_after:*) B=(sortkey_after:*)") + public Predicate r00_newestSortKey( + @Named("A") SortKeyPredicate.After a, @Named("B") SortKeyPredicate.After b) { + return a.getValue().compareTo(b.getValue()) >= 0 ? a : b; + } + + private static final class InvalidProvider implements Provider { + @Override + public T get() { + throw new OutOfScopeException("Not available at init"); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 4f681855eb..621ee98a3f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -110,7 +110,8 @@ public class ChangeQueryBuilder extends QueryBuilder { new QueryBuilder.Definition( ChangeQueryBuilder.class); - static class Arguments { + @VisibleForTesting + public static class Arguments { final Provider dbProvider; final Provider rewriter; final IdentifiedUser.GenericFactory userFactory; @@ -125,7 +126,8 @@ public class ChangeQueryBuilder extends QueryBuilder { final ChangeIndex index; @Inject - Arguments(Provider dbProvider, + @VisibleForTesting + public Arguments(Provider dbProvider, Provider rewriter, IdentifiedUser.GenericFactory userFactory, CapabilityControl.Factory capabilityControlFactory, @@ -161,7 +163,7 @@ public class ChangeQueryBuilder extends QueryBuilder { private boolean allowFileRegex; @Inject - ChangeQueryBuilder(Arguments args, @Assisted CurrentUser currentUser) { + public ChangeQueryBuilder(Arguments args, @Assisted CurrentUser currentUser) { super(mydef); this.args = args; this.currentUser = currentUser; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java index 2988021f96..1d6de6bc3c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java @@ -1,4 +1,4 @@ -// Copyright (C) 2009 The Android Open Source Project +// Copyright (C) 2013 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. @@ -14,701 +14,8 @@ package com.google.gerrit.server.query.change; -import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.server.ChangeAccess; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.query.IntPredicate; import com.google.gerrit.server.query.Predicate; -import com.google.gerrit.server.query.QueryRewriter; -import com.google.gerrit.server.query.RewritePredicate; -import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.ResultSet; -import com.google.inject.Inject; -import com.google.inject.OutOfScopeException; -import com.google.inject.Provider; -import com.google.inject.name.Named; -import java.util.Collection; - -public class ChangeQueryRewriter extends QueryRewriter { - private static final QueryRewriter.Definition mydef = - new QueryRewriter.Definition( - ChangeQueryRewriter.class, new ChangeQueryBuilder( - new ChangeQueryBuilder.Arguments( // - new InvalidProvider(), // - new InvalidProvider(), // - null, null, null, null, null, // - null, null, null, null, null), null)); - - private final Provider dbProvider; - private final IndexRewrite indexRewrite; - - @Inject - ChangeQueryRewriter(Provider dbProvider, - IndexRewrite indexRewrite) { - super(mydef); - this.dbProvider = dbProvider; - this.indexRewrite = indexRewrite; - } - - @Override - public Predicate and(Collection> l) { - return hasSource(l) ? new AndSource(dbProvider, l) : super.and(l); - } - - @Override - public Predicate or(Collection> l) { - return hasSource(l) ? new OrSource(l) : super.or(l); - } - - @Override - public Predicate preRewrite(Predicate in) { - return indexRewrite.rewrite(in); - } - - @Rewrite("-status:open") - @NoCostComputation - public Predicate r00_notOpen() { - return ChangeStatusPredicate.closed(dbProvider); - } - - @Rewrite("-status:closed") - @NoCostComputation - public Predicate r00_notClosed() { - return ChangeStatusPredicate.open(dbProvider); - } - - @SuppressWarnings("unchecked") - @NoCostComputation - @Rewrite("-status:merged") - public Predicate r00_notMerged() { - return or(ChangeStatusPredicate.open(dbProvider), - new ChangeStatusPredicate(dbProvider, Change.Status.ABANDONED)); - } - - @SuppressWarnings("unchecked") - @NoCostComputation - @Rewrite("-status:abandoned") - public Predicate r00_notAbandoned() { - return or(ChangeStatusPredicate.open(dbProvider), - new ChangeStatusPredicate(dbProvider, Change.Status.MERGED)); - } - - @SuppressWarnings("unchecked") - @NoCostComputation - @Rewrite("sortkey_before:z A=(age:*)") - public Predicate r00_ageToSortKey(@Named("A") AgePredicate a) { - String cut = ChangeUtil.sortKey(a.getCut(), Integer.MAX_VALUE); - return and(new SortKeyPredicate.Before(dbProvider, cut), a); - } - - @NoCostComputation - @Rewrite("A=(limit:*) B=(limit:*)") - public Predicate r00_smallestLimit( - @Named("A") IntPredicate a, - @Named("B") IntPredicate b) { - return a.intValue() <= b.intValue() ? a : b; - } - - @NoCostComputation - @Rewrite("A=(sortkey_before:*) B=(sortkey_before:*)") - public Predicate r00_oldestSortKey( - @Named("A") SortKeyPredicate.Before a, - @Named("B") SortKeyPredicate.Before b) { - return a.getValue().compareTo(b.getValue()) <= 0 ? a : b; - } - - @NoCostComputation - @Rewrite("A=(sortkey_after:*) B=(sortkey_after:*)") - public Predicate r00_newestSortKey( - @Named("A") SortKeyPredicate.After a, @Named("B") SortKeyPredicate.After b) { - return a.getValue().compareTo(b.getValue()) >= 0 ? a : b; - } - - @Rewrite("status:open P=(project:*) B=(ref:*)") - public Predicate r05_byBranchOpen( - @Named("P") final ProjectPredicate p, - @Named("B") final RefPredicate b) { - return new ChangeSource(500) { - @Override - ResultSet scan(ChangeAccess a) - throws OrmException { - return a.byBranchOpenAll( - new Branch.NameKey(p.getValueKey(), b.getValue())); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() - && p.match(cd) - && b.match(cd); - } - }; - } - - @Rewrite("status:merged P=(project:*) B=(ref:*) S=(sortkey_after:*) L=(limit:*)") - public Predicate r05_byBranchMergedPrev( - @Named("P") final ProjectPredicate p, - @Named("B") final RefPredicate b, - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byBranchClosedPrev(Change.Status.MERGED.getCode(), // - new Branch.NameKey(p.getValueKey(), b.getValue()), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && p.match(cd) // - && b.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:merged P=(project:*) B=(ref:*) S=(sortkey_before:*) L=(limit:*)") - public Predicate r05_byBranchMergedNext( - @Named("P") final ProjectPredicate p, - @Named("B") final RefPredicate b, - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byBranchClosedNext(Change.Status.MERGED.getCode(), // - new Branch.NameKey(p.getValueKey(), b.getValue()), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && p.match(cd) // - && b.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:open P=(project:*) S=(sortkey_after:*) L=(limit:*)") - public Predicate r10_byProjectOpenPrev( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(500, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectOpenPrev(p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() // - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:open P=(project:*) S=(sortkey_before:*) L=(limit:*)") - public Predicate r10_byProjectOpenNext( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(500, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectOpenNext(p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() // - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:merged P=(project:*) S=(sortkey_after:*) L=(limit:*)") - public Predicate r10_byProjectMergedPrev( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectClosedPrev(Change.Status.MERGED.getCode(), // - p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:merged P=(project:*) S=(sortkey_before:*) L=(limit:*)") - public Predicate r10_byProjectMergedNext( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectClosedNext(Change.Status.MERGED.getCode(), // - p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:abandoned P=(project:*) S=(sortkey_after:*) L=(limit:*)") - public Predicate r10_byProjectAbandonedPrev( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectClosedPrev(Change.Status.ABANDONED.getCode(), // - p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:abandoned P=(project:*) S=(sortkey_before:*) L=(limit:*)") - public Predicate r10_byProjectAbandonedNext( - @Named("P") final ProjectPredicate p, - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(40000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.byProjectClosedNext(Change.Status.ABANDONED.getCode(), // - p.getValueKey(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED - && p.match(cd) // - && s.match(cd); - } - }; - } - - @Rewrite("status:open S=(sortkey_after:*) L=(limit:*)") - public Predicate r20_byOpenPrev( - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(2000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allOpenPrev(key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() && s.match(cd); - } - }; - } - - @Rewrite("status:open S=(sortkey_before:*) L=(limit:*)") - public Predicate r20_byOpenNext( - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(2000, s.getValue(), l.intValue()) { - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allOpenNext(key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() && s.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:merged S=(sortkey_after:*) L=(limit:*)") - public Predicate r20_byMergedPrev( - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(50000, s.getValue(), l.intValue()) { - { - init("r20_byMergedPrev", s, l); - } - - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allClosedPrev(Change.Status.MERGED.getCode(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && s.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:merged S=(sortkey_before:*) L=(limit:*)") - public Predicate r20_byMergedNext( - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(50000, s.getValue(), l.intValue()) { - { - init("r20_byMergedNext", s, l); - } - - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allClosedNext(Change.Status.MERGED.getCode(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.MERGED - && s.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:abandoned S=(sortkey_after:*) L=(limit:*)") - public Predicate r20_byAbandonedPrev( - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(50000, s.getValue(), l.intValue()) { - { - init("r20_byAbandonedPrev", s, l); - } - - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allClosedPrev(Change.Status.ABANDONED.getCode(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED - && s.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:abandoned S=(sortkey_before:*) L=(limit:*)") - public Predicate r20_byAbandonedNext( - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return new PaginatedSource(50000, s.getValue(), l.intValue()) { - { - init("r20_byAbandonedNext", s, l); - } - - @Override - ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException { - return a.allClosedNext(Change.Status.ABANDONED.getCode(), key, limit); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED - && s.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:closed S=(sortkey_after:*) L=(limit:*)") - public Predicate r20_byClosedPrev( - @Named("S") final SortKeyPredicate.After s, - @Named("L") final IntPredicate l) { - return or(r20_byMergedPrev(s, l), r20_byAbandonedPrev(s, l)); - } - - @SuppressWarnings("unchecked") - @Rewrite("status:closed S=(sortkey_after:*) L=(limit:*)") - public Predicate r20_byClosedNext( - @Named("S") final SortKeyPredicate.Before s, - @Named("L") final IntPredicate l) { - return or(r20_byMergedNext(s, l), r20_byAbandonedNext(s, l)); - } - - @SuppressWarnings("unchecked") - @Rewrite("status:open O=(owner:*)") - public Predicate r25_byOwnerOpen( - @Named("O") final OwnerPredicate o) { - return new ChangeSource(50) { - { - init("r25_byOwnerOpen", o); - } - - @Override - ResultSet scan(ChangeAccess a) throws OrmException { - return a.byOwnerOpen(o.getAccountId()); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isOpen() && o.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:closed O=(owner:*)") - public Predicate r25_byOwnerClosed( - @Named("O") final OwnerPredicate o) { - return new ChangeSource(5000) { - { - init("r25_byOwnerClosed", o); - } - - @Override - ResultSet scan(ChangeAccess a) throws OrmException { - return a.byOwnerClosedAll(o.getAccountId()); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus().isClosed() && o.match(cd); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("O=(owner:*)") - public Predicate r26_byOwner(@Named("O") OwnerPredicate o) { - return or(r25_byOwnerOpen(o), r25_byOwnerClosed(o)); - } - - @SuppressWarnings("unchecked") - @Rewrite("status:open R=(reviewer:*)") - public Predicate r30_byReviewerOpen( - @Named("R") final ReviewerPredicate r) { - return new Source() { - { - init("r30_byReviewerOpen", r); - } - - @Override - public ResultSet read() throws OrmException { - return ChangeDataResultSet.patchSetApproval(dbProvider.get() - .patchSetApprovals().openByUser(r.getAccountId())); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - Change change = cd.change(dbProvider); - return change != null && change.getStatus().isOpen() && r.match(cd); - } - - @Override - public int getCardinality() { - return 50; - } - - @Override - public int getCost() { - return ChangeCosts.cost(ChangeCosts.APPROVALS_SCAN, getCardinality()); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("status:closed R=(reviewer:*)") - public Predicate r30_byReviewerClosed( - @Named("R") final ReviewerPredicate r) { - return new Source() { - { - init("r30_byReviewerClosed", r); - } - - @Override - public ResultSet read() throws OrmException { - return ChangeDataResultSet.patchSetApproval(dbProvider.get() - .patchSetApprovals().closedByUserAll(r.getAccountId())); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - Change change = cd.change(dbProvider); - return change != null && change.getStatus().isClosed() && r.match(cd); - } - - @Override - public int getCardinality() { - return 5000; - } - - @Override - public int getCost() { - return ChangeCosts.cost(ChangeCosts.APPROVALS_SCAN, getCardinality()); - } - }; - } - - @SuppressWarnings("unchecked") - @Rewrite("R=(reviewer:*)") - public Predicate r31_byReviewer( - @Named("R") final ReviewerPredicate r) { - return or(r30_byReviewerOpen(r), r30_byReviewerClosed(r)); - } - - @Rewrite("status:submitted") - public Predicate r99_allSubmitted() { - return new ChangeSource(50) { - @Override - ResultSet scan(ChangeAccess a) throws OrmException { - return a.allSubmitted(); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return cd.change(dbProvider).getStatus() == Change.Status.SUBMITTED; - } - }; - } - - @Rewrite("P=(project:*)") - public Predicate r99_byProject( - @Named("P") final ProjectPredicate p) { - return new ChangeSource(1000000) { - @Override - ResultSet scan(ChangeAccess a) throws OrmException { - return a.byProject(p.getValueKey()); - } - - @Override - public boolean match(ChangeData cd) throws OrmException { - return p.match(cd); - } - }; - } - - private static boolean hasSource(Collection> l) { - for (Predicate p : l) { - if (p instanceof ChangeDataSource) { - return true; - } - } - return false; - } - - private abstract static class Source extends RewritePredicate - implements ChangeDataSource { - @Override - public boolean hasChange() { - return false; - } - } - - private abstract class ChangeSource extends Source { - private final int cardinality; - - ChangeSource(int card) { - this.cardinality = card; - } - - abstract ResultSet scan(ChangeAccess a) throws OrmException; - - @Override - public ResultSet read() throws OrmException { - return ChangeDataResultSet.change(scan(dbProvider.get().changes())); - } - - @Override - public boolean hasChange() { - return true; - } - - @Override - public int getCardinality() { - return cardinality; - } - - @Override - public int getCost() { - return ChangeCosts.cost(ChangeCosts.CHANGES_SCAN, getCardinality()); - } - } - - private abstract class PaginatedSource extends ChangeSource implements - Paginated { - private final String startKey; - private final int limit; - - PaginatedSource(int card, String start, int lim) { - super(card); - this.startKey = start; - this.limit = lim; - } - - @Override - public int limit() { - return limit; - } - - @Override - ResultSet scan(ChangeAccess a) throws OrmException { - return scan(a, startKey, limit); - } - - @Override - public ResultSet restart(ChangeData last) throws OrmException { - return ChangeDataResultSet.change(scan(dbProvider.get().changes(), // - last.change(dbProvider).getSortKey(), // - limit)); - } - - abstract ResultSet scan(ChangeAccess a, String key, int limit) - throws OrmException; - } - - private static final class InvalidProvider implements Provider { - @Override - public T get() { - throw new OutOfScopeException("Not available at init"); - } - } +public interface ChangeQueryRewriter { + Predicate rewrite(Predicate in); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java index 6d08db078a..d5d6a92ca7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java @@ -47,7 +47,7 @@ public final class ChangeStatusPredicate extends IndexPredicate { VALUES = values.build(); } - static Predicate open(Provider dbProvider) { + public static Predicate open(Provider dbProvider) { List> r = new ArrayList>(4); for (final Change.Status e : Change.Status.values()) { if (e.isOpen()) { @@ -57,7 +57,7 @@ public final class ChangeStatusPredicate extends IndexPredicate { return r.size() == 1 ? r.get(0) : or(r); } - static Predicate closed(Provider dbProvider) { + public static Predicate closed(Provider dbProvider) { List> r = new ArrayList>(4); for (final Change.Status e : Change.Status.values()) { if (e.isClosed()) { @@ -83,7 +83,7 @@ public final class ChangeStatusPredicate extends IndexPredicate { this.status = status; } - Change.Status getStatus() { + public Change.Status getStatus() { return status; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java index 1437dad1d6..05d75737d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java @@ -55,9 +55,4 @@ class CommentPredicate extends IndexPredicate { public int getCost() { return 1; } - - @Override - public boolean isIndexOnly() { - return true; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java index 1601befda3..002dc99e12 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java @@ -54,9 +54,4 @@ class EqualsFilePredicate extends IndexPredicate { public int getCost() { return 1; } - - @Override - public boolean isIndexOnly() { - return true; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewrite.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewrite.java deleted file mode 100644 index 88c646baf9..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewrite.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (C) 2013 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.gerrit.server.query.Predicate; - -public interface IndexRewrite { - /** Instance indicating secondary index is disabled. */ - public static final IndexRewrite DISABLED = new IndexRewrite() { - @Override - public Predicate rewrite(Predicate in) { - return in; - } - }; - - /** - * Rewrite a predicate to push as much boolean logic as possible into the - * secondary index query system. - * - * @param in predicate to rewrite. - * @return a predicate with some subtrees replaced with predicates that are - * also sources that query the index directly. - */ - public Predicate rewrite(Predicate in); -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java deleted file mode 100644 index 51f620856d..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java +++ /dev/null @@ -1,222 +0,0 @@ -// Copyright (C) 2013 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.common.collect.Lists; -import com.google.common.collect.Sets; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.index.ChangeIndex; -import com.google.gerrit.server.index.IndexPredicate; -import com.google.gerrit.server.index.PredicateWrapper; -import com.google.gerrit.server.query.AndPredicate; -import com.google.gerrit.server.query.NotPredicate; -import com.google.gerrit.server.query.OrPredicate; -import com.google.gerrit.server.query.Predicate; -import com.google.gerrit.server.query.QueryParseException; -import com.google.inject.Inject; - -import java.util.BitSet; -import java.util.EnumSet; -import java.util.List; -import java.util.Set; - -/** Rewriter that pushes boolean logic into the secondary index. */ -public class IndexRewriteImpl implements IndexRewrite { - /** Set of all open change statuses. */ - public static final Set OPEN_STATUSES; - - /** Set of all closed change statuses. */ - public static final Set CLOSED_STATUSES; - - static { - EnumSet open = EnumSet.noneOf(Change.Status.class); - EnumSet closed = EnumSet.noneOf(Change.Status.class); - for (Change.Status s : Change.Status.values()) { - if (s.isOpen()) { - open.add(s); - } else { - closed.add(s); - } - } - OPEN_STATUSES = Sets.immutableEnumSet(open); - CLOSED_STATUSES = Sets.immutableEnumSet(closed); - } - - /** - * Get the set of statuses that changes matching the given predicate may have. - * - * @param in predicate - * @return the maximal set of statuses that any changes matching the input - * predicates may have, based on examining boolean and - * {@link ChangeStatusPredicate}s. - */ - public static EnumSet getPossibleStatus(Predicate in) { - if (in instanceof ChangeStatusPredicate) { - return EnumSet.of(((ChangeStatusPredicate) in).getStatus()); - } else if (in instanceof NotPredicate) { - return EnumSet.complementOf(getPossibleStatus(in.getChild(0))); - } else if (in instanceof OrPredicate) { - EnumSet s = EnumSet.noneOf(Change.Status.class); - for (int i = 0; i < in.getChildCount(); i++) { - s.addAll(getPossibleStatus(in.getChild(i))); - } - return s; - } else if (in instanceof AndPredicate) { - EnumSet s = EnumSet.allOf(Change.Status.class); - for (int i = 0; i < in.getChildCount(); i++) { - s.retainAll(getPossibleStatus(in.getChild(i))); - } - return s; - } else if (in.getChildCount() == 0) { - return EnumSet.allOf(Change.Status.class); - } else { - throw new IllegalStateException( - "Invalid predicate type in change index query: " + in.getClass()); - } - } - - private final ChangeIndex index; - - @Inject - IndexRewriteImpl(ChangeIndex index) { - this.index = index; - } - - @Override - public Predicate rewrite(Predicate in) { - Predicate out = rewriteImpl(in); - if (out == null) { - return in; - } else if (out == in) { - return wrap(out); - } else { - return out; - } - } - - /** - * Rewrite a single predicate subtree. - * - * @param in predicate to rewrite. - * @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 rewriteImpl(Predicate in) { - if (in instanceof IndexPredicate) { - return in; - } - if (!isBoolean(in)) { - return null; - } - int n = in.getChildCount(); - BitSet toKeep = new BitSet(n); - BitSet toWrap = new BitSet(n); - BitSet rewritten = new BitSet(n); - List> newChildren = Lists.newArrayListWithCapacity(n); - for (int i = 0; i < n; i++) { - Predicate c = in.getChild(i); - Predicate nc = rewriteImpl(c); - if (nc == null) { - toKeep.set(i); - newChildren.add(c); - } else if (nc == c) { - toWrap.set(i); - newChildren.add(nc); - } else { - rewritten.set(i); - newChildren.add(nc); - } - } - if (toKeep.cardinality() == n) { - return null; // Can't rewrite any children. - } - if (rewritten.cardinality() == n) { - // All children were partially, but not fully, rewritten. - return in.copy(newChildren); - } - if (toWrap.cardinality() == n) { - // All children can be fully rewritten, push work to parent. - return in; - } - return partitionChildren(in, newChildren, toWrap); - } - - - private Predicate partitionChildren(Predicate in, - List> newChildren, BitSet toWrap) { - if (toWrap.cardinality() == 1) { - int i = toWrap.nextSetBit(0); - newChildren.set(i, wrap(newChildren.get(i))); - return in.copy(newChildren); - } - - // Group all toWrap predicates into a wrapped subtree and place it as a - // sibling of the non-/partially-wrapped predicates. Assumes partitioning - // the children into arbitrary subtrees of the same type is logically - // equivalent to having them as siblings. - List> wrapped = Lists.newArrayListWithCapacity( - toWrap.cardinality()); - List> all = Lists.newArrayListWithCapacity( - newChildren.size() - toWrap.cardinality() + 1); - for (int i = 0; i < newChildren.size(); i++) { - Predicate child = newChildren.get(i); - if (toWrap.get(i)) { - wrapped.add(child); - if (allNonIndexOnly(child)) { - // Duplicate non-index-only predicate subtrees alongside the wrapped - // subtrees so they can provide index hints to the DB-based rewriter. - all.add(child); - } - } else { - all.add(child); - } - } - all.add(wrap(in.copy(wrapped))); - return in.copy(all); - } - - private static boolean allNonIndexOnly(Predicate p) { - if (p instanceof IndexPredicate) { - return !((IndexPredicate) p).isIndexOnly(); - } - if (isBoolean(p)) { - for (int i = 0; i < p.getChildCount(); i++) { - if (!allNonIndexOnly(p.getChild(i))) { - return false; - } - } - return true; - } else { - return true; - } - } - - private PredicateWrapper wrap(Predicate p) { - try { - return new PredicateWrapper(index, p); - } catch (QueryParseException e) { - throw new IllegalStateException( - "Failed to convert " + p + " to index predicate", e); - } - } - - private static boolean isBoolean(Predicate p) { - return p instanceof AndPredicate || p instanceof OrPredicate - || p instanceof NotPredicate; - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java index 514c29f483..62a3876ec5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java @@ -59,9 +59,4 @@ class MessagePredicate extends IndexPredicate { public int getCost() { return 1; } - - @Override - public boolean isIndexOnly() { - return true; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OrSource.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OrSource.java index ec5195b068..4f36777401 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OrSource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OrSource.java @@ -25,10 +25,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; -class OrSource extends OrPredicate implements ChangeDataSource { +public class OrSource extends OrPredicate implements ChangeDataSource { private int cardinality = -1; - OrSource(final Collection> that) { + public OrSource(Collection> that) { super(that); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java new file mode 100644 index 0000000000..78b3c95c8b --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SqlRewriterImpl.java @@ -0,0 +1,631 @@ +// Copyright (C) 2009 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.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.server.ChangeAccess; +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.gerrit.server.query.RewritePredicate; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.name.Named; + +import java.util.Collection; + +public class SqlRewriterImpl extends BasicChangeRewrites + implements ChangeQueryRewriter { + private static final QueryRewriter.Definition mydef = + new QueryRewriter.Definition( + SqlRewriterImpl.class, BUILDER); + + @Inject + SqlRewriterImpl(Provider dbProvider) { + super(mydef, dbProvider); + } + + @Override + public Predicate and(Collection> l) { + return hasSource(l) ? new AndSource(dbProvider, l) : super.and(l); + } + + @Override + public Predicate or(Collection> l) { + return hasSource(l) ? new OrSource(l) : super.or(l); + } + + @Rewrite("status:open P=(project:*) B=(ref:*)") + public Predicate r05_byBranchOpen( + @Named("P") final ProjectPredicate p, + @Named("B") final RefPredicate b) { + return new ChangeSource(500) { + @Override + ResultSet scan(ChangeAccess a) + throws OrmException { + return a.byBranchOpenAll( + new Branch.NameKey(p.getValueKey(), b.getValue())); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() + && p.match(cd) + && b.match(cd); + } + }; + } + + @Rewrite("status:merged P=(project:*) B=(ref:*) S=(sortkey_after:*) L=(limit:*)") + public Predicate r05_byBranchMergedPrev( + @Named("P") final ProjectPredicate p, + @Named("B") final RefPredicate b, + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byBranchClosedPrev(Change.Status.MERGED.getCode(), // + new Branch.NameKey(p.getValueKey(), b.getValue()), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && p.match(cd) // + && b.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:merged P=(project:*) B=(ref:*) S=(sortkey_before:*) L=(limit:*)") + public Predicate r05_byBranchMergedNext( + @Named("P") final ProjectPredicate p, + @Named("B") final RefPredicate b, + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byBranchClosedNext(Change.Status.MERGED.getCode(), // + new Branch.NameKey(p.getValueKey(), b.getValue()), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && p.match(cd) // + && b.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:open P=(project:*) S=(sortkey_after:*) L=(limit:*)") + public Predicate r10_byProjectOpenPrev( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(500, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectOpenPrev(p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() // + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:open P=(project:*) S=(sortkey_before:*) L=(limit:*)") + public Predicate r10_byProjectOpenNext( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(500, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectOpenNext(p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() // + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:merged P=(project:*) S=(sortkey_after:*) L=(limit:*)") + public Predicate r10_byProjectMergedPrev( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectClosedPrev(Change.Status.MERGED.getCode(), // + p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:merged P=(project:*) S=(sortkey_before:*) L=(limit:*)") + public Predicate r10_byProjectMergedNext( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectClosedNext(Change.Status.MERGED.getCode(), // + p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:abandoned P=(project:*) S=(sortkey_after:*) L=(limit:*)") + public Predicate r10_byProjectAbandonedPrev( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectClosedPrev(Change.Status.ABANDONED.getCode(), // + p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:abandoned P=(project:*) S=(sortkey_before:*) L=(limit:*)") + public Predicate r10_byProjectAbandonedNext( + @Named("P") final ProjectPredicate p, + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(40000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.byProjectClosedNext(Change.Status.ABANDONED.getCode(), // + p.getValueKey(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED + && p.match(cd) // + && s.match(cd); + } + }; + } + + @Rewrite("status:open S=(sortkey_after:*) L=(limit:*)") + public Predicate r20_byOpenPrev( + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(2000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allOpenPrev(key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() && s.match(cd); + } + }; + } + + @Rewrite("status:open S=(sortkey_before:*) L=(limit:*)") + public Predicate r20_byOpenNext( + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(2000, s.getValue(), l.intValue()) { + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allOpenNext(key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() && s.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:merged S=(sortkey_after:*) L=(limit:*)") + public Predicate r20_byMergedPrev( + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(50000, s.getValue(), l.intValue()) { + { + init("r20_byMergedPrev", s, l); + } + + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allClosedPrev(Change.Status.MERGED.getCode(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && s.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:merged S=(sortkey_before:*) L=(limit:*)") + public Predicate r20_byMergedNext( + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(50000, s.getValue(), l.intValue()) { + { + init("r20_byMergedNext", s, l); + } + + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allClosedNext(Change.Status.MERGED.getCode(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.MERGED + && s.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:abandoned S=(sortkey_after:*) L=(limit:*)") + public Predicate r20_byAbandonedPrev( + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(50000, s.getValue(), l.intValue()) { + { + init("r20_byAbandonedPrev", s, l); + } + + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allClosedPrev(Change.Status.ABANDONED.getCode(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED + && s.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:abandoned S=(sortkey_before:*) L=(limit:*)") + public Predicate r20_byAbandonedNext( + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return new PaginatedSource(50000, s.getValue(), l.intValue()) { + { + init("r20_byAbandonedNext", s, l); + } + + @Override + ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException { + return a.allClosedNext(Change.Status.ABANDONED.getCode(), key, limit); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.ABANDONED + && s.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:closed S=(sortkey_after:*) L=(limit:*)") + public Predicate r20_byClosedPrev( + @Named("S") final SortKeyPredicate.After s, + @Named("L") final IntPredicate l) { + return or(r20_byMergedPrev(s, l), r20_byAbandonedPrev(s, l)); + } + + @SuppressWarnings("unchecked") + @Rewrite("status:closed S=(sortkey_after:*) L=(limit:*)") + public Predicate r20_byClosedNext( + @Named("S") final SortKeyPredicate.Before s, + @Named("L") final IntPredicate l) { + return or(r20_byMergedNext(s, l), r20_byAbandonedNext(s, l)); + } + + @SuppressWarnings("unchecked") + @Rewrite("status:open O=(owner:*)") + public Predicate r25_byOwnerOpen( + @Named("O") final OwnerPredicate o) { + return new ChangeSource(50) { + { + init("r25_byOwnerOpen", o); + } + + @Override + ResultSet scan(ChangeAccess a) throws OrmException { + return a.byOwnerOpen(o.getAccountId()); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isOpen() && o.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:closed O=(owner:*)") + public Predicate r25_byOwnerClosed( + @Named("O") final OwnerPredicate o) { + return new ChangeSource(5000) { + { + init("r25_byOwnerClosed", o); + } + + @Override + ResultSet scan(ChangeAccess a) throws OrmException { + return a.byOwnerClosedAll(o.getAccountId()); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus().isClosed() && o.match(cd); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("O=(owner:*)") + public Predicate r26_byOwner(@Named("O") OwnerPredicate o) { + return or(r25_byOwnerOpen(o), r25_byOwnerClosed(o)); + } + + @SuppressWarnings("unchecked") + @Rewrite("status:open R=(reviewer:*)") + public Predicate r30_byReviewerOpen( + @Named("R") final ReviewerPredicate r) { + return new Source() { + { + init("r30_byReviewerOpen", r); + } + + @Override + public ResultSet read() throws OrmException { + return ChangeDataResultSet.patchSetApproval(dbProvider.get() + .patchSetApprovals().openByUser(r.getAccountId())); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + Change change = cd.change(dbProvider); + return change != null && change.getStatus().isOpen() && r.match(cd); + } + + @Override + public int getCardinality() { + return 50; + } + + @Override + public int getCost() { + return ChangeCosts.cost(ChangeCosts.APPROVALS_SCAN, getCardinality()); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("status:closed R=(reviewer:*)") + public Predicate r30_byReviewerClosed( + @Named("R") final ReviewerPredicate r) { + return new Source() { + { + init("r30_byReviewerClosed", r); + } + + @Override + public ResultSet read() throws OrmException { + return ChangeDataResultSet.patchSetApproval(dbProvider.get() + .patchSetApprovals().closedByUserAll(r.getAccountId())); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + Change change = cd.change(dbProvider); + return change != null && change.getStatus().isClosed() && r.match(cd); + } + + @Override + public int getCardinality() { + return 5000; + } + + @Override + public int getCost() { + return ChangeCosts.cost(ChangeCosts.APPROVALS_SCAN, getCardinality()); + } + }; + } + + @SuppressWarnings("unchecked") + @Rewrite("R=(reviewer:*)") + public Predicate r31_byReviewer( + @Named("R") final ReviewerPredicate r) { + return or(r30_byReviewerOpen(r), r30_byReviewerClosed(r)); + } + + @Rewrite("status:submitted") + public Predicate r99_allSubmitted() { + return new ChangeSource(50) { + @Override + ResultSet scan(ChangeAccess a) throws OrmException { + return a.allSubmitted(); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return cd.change(dbProvider).getStatus() == Change.Status.SUBMITTED; + } + }; + } + + @Rewrite("P=(project:*)") + public Predicate r99_byProject( + @Named("P") final ProjectPredicate p) { + return new ChangeSource(1000000) { + @Override + ResultSet scan(ChangeAccess a) throws OrmException { + return a.byProject(p.getValueKey()); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + return p.match(cd); + } + }; + } + + private static boolean hasSource(Collection> l) { + for (Predicate p : l) { + if (p instanceof ChangeDataSource) { + return true; + } + } + return false; + } + + private abstract static class Source extends RewritePredicate + implements ChangeDataSource { + @Override + public boolean hasChange() { + return false; + } + } + + private abstract class ChangeSource extends Source { + private final int cardinality; + + ChangeSource(int card) { + this.cardinality = card; + } + + abstract ResultSet scan(ChangeAccess a) throws OrmException; + + @Override + public ResultSet read() throws OrmException { + return ChangeDataResultSet.change(scan(dbProvider.get().changes())); + } + + @Override + public boolean hasChange() { + return true; + } + + @Override + public int getCardinality() { + return cardinality; + } + + @Override + public int getCost() { + return ChangeCosts.cost(ChangeCosts.CHANGES_SCAN, getCardinality()); + } + } + + private abstract class PaginatedSource extends ChangeSource implements + Paginated { + private final String startKey; + private final int limit; + + PaginatedSource(int card, String start, int lim) { + super(card); + this.startKey = start; + this.limit = lim; + } + + @Override + public int limit() { + return limit; + } + + @Override + ResultSet scan(ChangeAccess a) throws OrmException { + return scan(a, startKey, limit); + } + + @Override + public ResultSet restart(ChangeData last) throws OrmException { + return ChangeDataResultSet.change(scan(dbProvider.get().changes(), // + last.change(dbProvider).getSortKey(), // + limit)); + } + + abstract ResultSet scan(ChangeAccess a, String key, int limit) + throws OrmException; + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java similarity index 78% rename from gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java rename to gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java index a754524761..6a7936d0bc 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.gerrit.server.query.change; +package com.google.gerrit.server.index; import static com.google.gerrit.reviewdb.client.Change.Status.ABANDONED; import static com.google.gerrit.reviewdb.client.Change.Status.DRAFT; @@ -23,13 +23,14 @@ 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.index.ChangeIndex; -import com.google.gerrit.server.index.PredicateWrapper; -import com.google.gerrit.server.query.AndPredicate; import com.google.gerrit.server.query.OperatorPredicate; -import com.google.gerrit.server.query.OrPredicate; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; +import com.google.gerrit.server.query.change.AndSource; +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 com.google.gerrit.server.query.change.OrSource; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; @@ -64,7 +65,7 @@ public class IndexRewriteTest extends TestCase { @Override public ChangeDataSource getSource(Predicate p) throws QueryParseException { - return new Source(); + return new Source(p); } @Override @@ -74,6 +75,12 @@ public class IndexRewriteTest extends TestCase { } private static class Source implements ChangeDataSource { + private final Predicate p; + + Source(Predicate p) { + this.p = p; + } + @Override public int getCardinality() { throw new UnsupportedOperationException(); @@ -88,6 +95,11 @@ public class IndexRewriteTest extends TestCase { public ResultSet read() throws OrmException { throw new UnsupportedOperationException(); } + + @Override + public String toString() { + return p.toString(); + } } public static class QueryBuilder extends ChangeQueryBuilder { @@ -127,19 +139,22 @@ public class IndexRewriteTest extends TestCase { private DummyIndex index; private ChangeQueryBuilder queryBuilder; - private IndexRewrite rewrite; + private IndexRewriteImpl rewrite; @Override public void setUp() throws Exception { super.setUp(); index = new DummyIndex(); queryBuilder = new QueryBuilder(); - rewrite = new IndexRewriteImpl(index); + rewrite = new IndexRewriteImpl( + index, + null, + new IndexRewriteImpl.BasicRewritesImpl(null)); } public void testIndexPredicate() throws Exception { Predicate in = parse("file:a"); - assertEquals(wrap(in), rewrite(in)); + assertEquals(query(in), rewrite(in)); } public void testNonIndexPredicate() throws Exception { @@ -149,33 +164,37 @@ public class IndexRewriteTest extends TestCase { public void testIndexPredicates() throws Exception { Predicate in = parse("file:a file:b"); - assertEquals(wrap(in), rewrite(in)); + assertEquals(query(in), rewrite(in)); } public void testNonIndexPredicates() throws Exception { Predicate in = parse("foo:a OR foo:b"); - assertSame(in, rewrite(in)); + assertEquals(in, rewrite(in)); } public void testOneIndexPredicate() throws Exception { Predicate in = parse("foo:a file:b"); Predicate out = rewrite(in); - assertSame(AndPredicate.class, out.getClass()); - assertEquals(ImmutableList.of(in.getChild(0), wrap(in.getChild(1))), + assertSame(AndSource.class, out.getClass()); + assertEquals( + ImmutableList.of(query(in.getChild(1)), in.getChild(0)), out.getChildren()); } public void testThreeLevelTreeWithAllIndexPredicates() throws Exception { Predicate in = parse("-status:abandoned (status:open OR status:merged)"); - assertEquals(wrap(in), rewrite.rewrite(in)); + assertEquals( + query(parse("status:new OR status:submitted OR status:draft OR status:merged")), + rewrite.rewrite(in)); } public void testThreeLevelTreeWithSomeIndexPredicates() throws Exception { Predicate in = parse("-foo:a (file:b OR file:c)"); Predicate out = rewrite(in); - assertEquals(AndPredicate.class, out.getClass()); - assertEquals(ImmutableList.of(in.getChild(0), wrap(in.getChild(1))), + assertEquals(AndSource.class, out.getClass()); + assertEquals( + ImmutableList.of(query(in.getChild(1)), in.getChild(0)), out.getChildren()); } @@ -183,20 +202,20 @@ public class IndexRewriteTest extends TestCase { Predicate in = parse("file:a OR foo:b OR file:c OR foo:d"); Predicate out = rewrite(in); - assertSame(OrPredicate.class, out.getClass()); + assertSame(OrSource.class, out.getClass()); assertEquals(ImmutableList.of( - in.getChild(1), in.getChild(3), - wrap(Predicate.or(in.getChild(0), in.getChild(2)))), + query(Predicate.or(in.getChild(0), in.getChild(2))), + in.getChild(1), in.getChild(3)), out.getChildren()); } - public void testDuplicateSimpleNonIndexOnlyPredicates() throws Exception { + public void testIndexAndNonIndexPredicates() throws Exception { Predicate in = parse("status:new bar:p file:a"); Predicate out = rewrite(in); - assertSame(AndPredicate.class, out.getClass()); + assertSame(AndSource.class, out.getClass()); assertEquals(ImmutableList.of( - in.getChild(0), in.getChild(1), - wrap(Predicate.and(in.getChild(0), in.getChild(2)))), + query(Predicate.and(in.getChild(0), in.getChild(2))), + in.getChild(1)), out.getChildren()); } @@ -204,10 +223,10 @@ public class IndexRewriteTest extends TestCase { Predicate in = parse("(status:new OR status:draft) bar:p file:a"); Predicate out = rewrite(in); - assertSame(AndPredicate.class, out.getClass()); + assertSame(AndSource.class, out.getClass()); assertEquals(ImmutableList.of( - in.getChild(0), in.getChild(1), - wrap(Predicate.and(in.getChild(0), in.getChild(2)))), + query(Predicate.and(in.getChild(0), in.getChild(2))), + in.getChild(1)), out.getChildren()); } @@ -215,10 +234,10 @@ public class IndexRewriteTest extends TestCase { Predicate in = parse("(status:new OR file:a) bar:p file:b"); Predicate out = rewrite(in); - assertSame(AndPredicate.class, out.getClass()); + assertSame(AndSource.class, out.getClass()); assertEquals(ImmutableList.of( - in.getChild(1), - wrap(Predicate.and(in.getChild(0), in.getChild(2)))), + query(Predicate.and(in.getChild(0), in.getChild(2))), + in.getChild(1)), out.getChildren()); } @@ -246,9 +265,9 @@ public class IndexRewriteTest extends TestCase { return rewrite.rewrite(in); } - private PredicateWrapper wrap(Predicate p) + private IndexedChangeQuery query(Predicate p) throws QueryParseException { - return new PredicateWrapper(index, p); + return new IndexedChangeQuery(index, p); } private Set status(String query) throws QueryParseException { diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java index c28a0fc3e5..28d1048400 100644 --- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java +++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java @@ -14,8 +14,8 @@ package com.google.gerrit.solr; -import static com.google.gerrit.server.query.change.IndexRewriteImpl.CLOSED_STATUSES; -import static com.google.gerrit.server.query.change.IndexRewriteImpl.OPEN_STATUSES; +import static com.google.gerrit.server.index.IndexRewriteImpl.CLOSED_STATUSES; +import static com.google.gerrit.server.index.IndexRewriteImpl.OPEN_STATUSES; import static com.google.gerrit.solr.IndexVersionCheck.SCHEMA_VERSIONS; import static com.google.gerrit.solr.IndexVersionCheck.solrIndexConfig; @@ -34,11 +34,11 @@ import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.FieldDef.FillArgs; import com.google.gerrit.server.index.FieldType; +import com.google.gerrit.server.index.IndexRewriteImpl; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeDataSource; -import com.google.gerrit.server.query.change.IndexRewriteImpl; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; @@ -224,6 +224,11 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener { return false; } + @Override + public String toString() { + return query.getQuery(); + } + @Override public ResultSet read() throws OrmException { try {