Throw QueryParseException earlier from index queries
In Ia839eb21 due to sort key predicate rewriting we pushed the call to
ChangeIndex.getSource() to be lazy when reading the predicate.
Wrapping a resulting QueryParseException means that exception text,
which is intended to be user-visible, isn't. Instead, send the query
to the secondary index implementation earlier, and just re-process the
query if the sort key is rewritten.
This change means it is definitively inappropriate to kick off
searches in the background when getSource() is called as opposed to
when read() is called on the resulting source; document this fact in
the getSource() javadoc.
Change-Id: I8fb2ab25aaadf4eb0eba44c9eb31351be0748c0e
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
index fd37cfb..397b044 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
@@ -125,6 +125,11 @@
/**
* Convert the given operator predicate into a source searching the index and
* returning only the documents matching that predicate.
+ * <p>
+ * This method may be called multiple times for variations on the same
+ * predicate or multiple predicate subtrees in the course of processing a
+ * single query, so it should not have any side effects (e.g. starting a
+ * search in the background).
*
* @param p the predicate to match. Must be a tree containing only AND, OR,
* or NOT predicates as internal nodes, and {@link IndexPredicate}s as
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java
index 2185af3..f915dad 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java
@@ -91,8 +91,9 @@
Predicate<ChangeData> pred, int limit) throws QueryParseException {
this.db = db;
this.index = index;
- this.pred = pred;
this.limit = limit;
+ this.pred = pred;
+ this.source = index.getSource(pred, limit);
}
@Override
@@ -131,14 +132,8 @@
@Override
public ResultSet<ChangeData> read() throws OrmException {
- final ChangeDataSource currSource;
- try {
- currSource = index.getSource(pred, limit);
- } catch (QueryParseException e) {
- throw new OrmException(e);
- }
- source = currSource;
- final ResultSet<ChangeData> rs = source.read();
+ final ChangeDataSource currSource = source;
+ final ResultSet<ChangeData> rs = currSource.read();
return new ResultSet<ChangeData>() {
@Override
@@ -174,6 +169,14 @@
@Override
public ResultSet<ChangeData> restart(ChangeData last) throws OrmException {
pred = replaceSortKeyPredicates(pred, last.change(db).getSortKey());
+ try {
+ source = index.getSource(pred, limit);
+ } catch (QueryParseException e) {
+ // Don't need to show this exception to the user; the only thing that
+ // changed about pred was its SortKeyPredicates, and any other QPEs
+ // that might happen should have already thrown from the constructor.
+ throw new OrmException(e);
+ }
return read();
}
