Assume status:open if the query isn't executable
Rather than throwing back an obtuse error about not being able to execute a query string, add in the status:open and retry to plan the query with the rewriter. It should succeed this time and give us a data source that can actually evaluate. We assume status:open because uses are primarily interested in the open changes when they enter a query, and the set that is open is smaller than the set that is closed. Users can also pretty quickly realize we are only showing them open changes and modify their query if they needed to search closed ones. Change-Id: Ieaf78bde3201767eaeb973c015785413597e409a Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
@@ -156,12 +156,17 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
|
||||
builder.limit(limit), //
|
||||
visibleToMe //
|
||||
);
|
||||
q = queryRewriter.get().rewrite(q);
|
||||
if (q instanceof ChangeDataSource) {
|
||||
ChangeDataSource ds = (ChangeDataSource) q;
|
||||
|
||||
ChangeQueryRewriter rewriter = queryRewriter.get();
|
||||
Predicate<ChangeData> s = rewriter.rewrite(q);
|
||||
if (!(s instanceof ChangeDataSource)) {
|
||||
s = rewriter.rewrite(Predicate.and(builder.status_open(), q));
|
||||
}
|
||||
|
||||
if (s instanceof ChangeDataSource) {
|
||||
ArrayList<Change> r = new ArrayList();
|
||||
HashSet<Change.Id> want = new HashSet<Change.Id>();
|
||||
for (ChangeData d : ds.read()) {
|
||||
for (ChangeData d : ((ChangeDataSource) s).read()) {
|
||||
if (d.hasChange()) {
|
||||
// Checking visibleToMe here should be unnecessary, the
|
||||
// query should have already performed it. But we don't
|
||||
@@ -190,7 +195,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
|
||||
Collections.sort(r, cmp);
|
||||
return new ListResultSet<Change>(r);
|
||||
} else {
|
||||
throw new InvalidQueryException("Not Supported", q.toString());
|
||||
throw new InvalidQueryException("Not Supported", s.toString());
|
||||
}
|
||||
} catch (QueryParseException e) {
|
||||
throw new InvalidQueryException(e.getMessage(), query);
|
||||
|
||||
@@ -136,7 +136,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
@Operator
|
||||
public Predicate<ChangeData> status(String statusName) {
|
||||
if ("open".equals(statusName)) {
|
||||
return ChangeStatusPredicate.open(dbProvider);
|
||||
return status_open();
|
||||
|
||||
} else if ("closed".equals(statusName)) {
|
||||
return ChangeStatusPredicate.closed(dbProvider);
|
||||
@@ -149,6 +149,10 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
}
|
||||
}
|
||||
|
||||
public Predicate<ChangeData> status_open() {
|
||||
return ChangeStatusPredicate.open(dbProvider);
|
||||
}
|
||||
|
||||
@Operator
|
||||
public Predicate<ChangeData> has(String value) {
|
||||
if ("star".equalsIgnoreCase(value)) {
|
||||
|
||||
Reference in New Issue
Block a user