Merge "Add HasCardinality interface which helps in defining a cardinality" into stable-3.4

17 files changed, 133 insertions, 18 deletions

diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index eb89681c6e..187c83bbc5 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -42,6 +42,7 @@ import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.index.query.ListResultSet;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
@@ -351,11 +352,13 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
 
   protected class ElasticQuerySource implements DataSource<V> {
     private final QueryOptions opts;
+    private final Predicate<V> predicate;
     private final String search;
 
     ElasticQuerySource(Predicate<V> p, QueryOptions opts, JsonArray sortArray)
         throws QueryParseException {
       this.opts = opts;
+      this.predicate = p;
       QueryBuilder qb = queryBuilder.toQueryBuilder(p);
       SearchSourceBuilder searchSource =
           new SearchSourceBuilder(client.adapter())
@@ -371,6 +374,9 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
 
     @Override
     public int getCardinality() {
+      if (predicate instanceof HasCardinality) {
+        return ((HasCardinality) predicate).getCardinality();
+      }
       return 10;
     }
 
diff --git a/java/com/google/gerrit/index/query/DataSource.java b/java/com/google/gerrit/index/query/DataSource.java
index 2c2ba532df..087d1fe1ea 100644
--- a/java/com/google/gerrit/index/query/DataSource.java
+++ b/java/com/google/gerrit/index/query/DataSource.java
@@ -14,10 +14,7 @@
 
 package com.google.gerrit.index.query;
 
-public interface DataSource<T> {
-  /** @return an estimate of the number of results from {@link #read()}. */
-  int getCardinality();
-
+public interface DataSource<T> extends HasCardinality {
   /** @return read from the database and return the results. */
   ResultSet<T> read();
 
diff --git a/java/com/google/gerrit/index/query/HasCardinality.java b/java/com/google/gerrit/index/query/HasCardinality.java
new file mode 100644
index 0000000000..ac6640a5ba
--- /dev/null
+++ b/java/com/google/gerrit/index/query/HasCardinality.java
@@ -0,0 +1,20 @@
+// Copyright (C) 2022 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.index.query;
+
+public interface HasCardinality {
+  /** @return an estimate of the number of results a source can return. */
+  int getCardinality();
+}
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index bee2f17390..542b9084a3 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -53,6 +53,7 @@ import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.RefState;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.FieldBundle;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.ResultSet;
@@ -382,7 +383,10 @@ public class LuceneChangeIndex implements ChangeIndex {
 
     @Override
     public int getCardinality() {
-      return 10; // TODO(dborowitz): estimate from Lucene?
+      if (predicate instanceof HasCardinality) {
+        return ((HasCardinality) predicate).getCardinality();
+      }
+      return 10;
     }
 
     @Override
diff --git a/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java
index 05cc6ca1e2..f837ef47f9 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java
@@ -15,10 +15,11 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
 /** Predicate over Change-Id strings (aka Change.Key). */
-public class ChangeIdPredicate extends ChangeIndexPredicate {
+public class ChangeIdPredicate extends ChangeIndexPredicate implements HasCardinality {
   public ChangeIdPredicate(String id) {
     super(ChangeField.ID, ChangeQueryBuilder.FIELD_CHANGE, id);
   }
@@ -41,4 +42,9 @@ public class ChangeIdPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 5;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
index 88e93d9d25..ef902b58e7 100644
--- a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
@@ -20,6 +20,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Change.Status;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.server.index.change.ChangeField;
 import java.util.ArrayList;
@@ -38,7 +39,7 @@ import java.util.TreeMap;
  *
  * <p>Status names are looked up by prefix case-insensitively.
  */
-public final class ChangeStatusPredicate extends ChangeIndexPredicate {
+public final class ChangeStatusPredicate extends ChangeIndexPredicate implements HasCardinality {
   private static final String INVALID_STATUS = "__invalid__";
   static final Predicate<ChangeData> NONE = new ChangeStatusPredicate(null);
 
@@ -143,4 +144,19 @@ public final class ChangeStatusPredicate extends ChangeIndexPredicate {
   public String toString() {
     return getOperator() + ":" + getValue();
   }
+
+  @Override
+  public int getCardinality() {
+    if (getStatus() == null) {
+      return 0;
+    }
+    switch (getStatus()) {
+      case MERGED:
+        return 50_000;
+      case ABANDONED:
+        return 50_000;
+      default:
+        return 2000;
+    }
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/CommitPredicate.java b/java/com/google/gerrit/server/query/change/CommitPredicate.java
index b54ee64441..970c222e19 100644
--- a/java/com/google/gerrit/server/query/change/CommitPredicate.java
+++ b/java/com/google/gerrit/server/query/change/CommitPredicate.java
@@ -21,8 +21,9 @@ import static com.google.gerrit.server.index.change.ChangeField.EXACT_COMMIT;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.git.ObjectIds;
 import com.google.gerrit.index.FieldDef;
+import com.google.gerrit.index.query.HasCardinality;
 
-public class CommitPredicate extends ChangeIndexPredicate {
+public class CommitPredicate extends ChangeIndexPredicate implements HasCardinality {
   static FieldDef<ChangeData, ?> commitField(String id) {
     if (id.length() == ObjectIds.STR_LEN) {
       return EXACT_COMMIT;
@@ -56,4 +57,9 @@ public class CommitPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 5;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java b/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
index 6d1576fdd1..f2bc45d23e 100644
--- a/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
+++ b/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
@@ -15,9 +15,10 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class HasDraftByPredicate extends ChangeIndexPredicate {
+public class HasDraftByPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected final Account.Id accountId;
 
   public HasDraftByPredicate(Account.Id accountId) {
@@ -34,4 +35,9 @@ public class HasDraftByPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 20;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java b/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java
index d531236286..5dd780b300 100644
--- a/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java
@@ -17,9 +17,10 @@ package com.google.gerrit.server.query.change;
 import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
 
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.HasCardinality;
 
 /** Predicate over change number (aka legacy ID or Change.Id). */
-public class LegacyChangeIdPredicate extends ChangeIndexPredicate {
+public class LegacyChangeIdPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected final Change.Id id;
 
   public LegacyChangeIdPredicate(Change.Id id) {
@@ -36,4 +37,9 @@ public class LegacyChangeIdPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 1;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java b/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java
index bae9c0d1fd..be0aae3991 100644
--- a/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java
@@ -17,9 +17,10 @@ package com.google.gerrit.server.query.change;
 import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
 
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.HasCardinality;
 
 /** Predicate over change number (aka legacy ID or Change.Id). */
-public class LegacyChangeIdStrPredicate extends ChangeIndexPredicate {
+public class LegacyChangeIdStrPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected final Change.Id id;
 
   public LegacyChangeIdStrPredicate(Change.Id id) {
@@ -36,4 +37,9 @@ public class LegacyChangeIdStrPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 1;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/OwnerPredicate.java b/java/com/google/gerrit/server/query/change/OwnerPredicate.java
index 923a9ca924..175a27309b 100644
--- a/java/com/google/gerrit/server/query/change/OwnerPredicate.java
+++ b/java/com/google/gerrit/server/query/change/OwnerPredicate.java
@@ -16,9 +16,10 @@ package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class OwnerPredicate extends ChangeIndexPredicate {
+public class OwnerPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected final Account.Id id;
 
   public OwnerPredicate(Account.Id id) {
@@ -40,4 +41,9 @@ public class OwnerPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 5000;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/ProjectPredicate.java b/java/com/google/gerrit/server/query/change/ProjectPredicate.java
index db5a9327f0..99286fa67c 100644
--- a/java/com/google/gerrit/server/query/change/ProjectPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ProjectPredicate.java
@@ -16,9 +16,10 @@ package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class ProjectPredicate extends ChangeIndexPredicate {
+public class ProjectPredicate extends ChangeIndexPredicate implements HasCardinality {
   public ProjectPredicate(String id) {
     super(ChangeField.PROJECT, id);
   }
@@ -42,4 +43,9 @@ public class ProjectPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 1_000_000;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/RefPredicate.java b/java/com/google/gerrit/server/query/change/RefPredicate.java
index 1c70a62d9f..715b527fba 100644
--- a/java/com/google/gerrit/server/query/change/RefPredicate.java
+++ b/java/com/google/gerrit/server/query/change/RefPredicate.java
@@ -15,9 +15,10 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class RefPredicate extends ChangeIndexPredicate {
+public class RefPredicate extends ChangeIndexPredicate implements HasCardinality {
   public RefPredicate(String ref) {
     super(ChangeField.REF, ref);
   }
@@ -35,4 +36,9 @@ public class RefPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 10_000;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/RevertOfPredicate.java b/java/com/google/gerrit/server/query/change/RevertOfPredicate.java
index eea1b1e232..5fbaac9e07 100644
--- a/java/com/google/gerrit/server/query/change/RevertOfPredicate.java
+++ b/java/com/google/gerrit/server/query/change/RevertOfPredicate.java
@@ -14,9 +14,10 @@
 
 package com.google.gerrit.server.query.change;
 
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class RevertOfPredicate extends ChangeIndexPredicate {
+public class RevertOfPredicate extends ChangeIndexPredicate implements HasCardinality {
   public RevertOfPredicate(String revertOf) {
     super(ChangeField.REVERT_OF, revertOf);
   }
@@ -33,4 +34,9 @@ public class RevertOfPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 1;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/ReviewerPredicate.java b/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
index d783f76aa5..237b986423 100644
--- a/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
@@ -17,11 +17,12 @@ package com.google.gerrit.server.query.change;
 import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 
-public class ReviewerPredicate extends ChangeIndexPredicate {
+public class ReviewerPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected static Predicate<ChangeData> forState(Account.Id id, ReviewerStateInternal state) {
     checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer");
     return new ReviewerPredicate(state, id);
@@ -57,4 +58,9 @@ public class ReviewerPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 5000;
+  }
 }
diff --git a/java/com/google/gerrit/server/query/change/StarPredicate.java b/java/com/google/gerrit/server/query/change/StarPredicate.java
index 788f1a32f6..ce13da918d 100644
--- a/java/com/google/gerrit/server/query/change/StarPredicate.java
+++ b/java/com/google/gerrit/server/query/change/StarPredicate.java
@@ -15,10 +15,11 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class StarPredicate extends ChangeIndexPredicate {
+public class StarPredicate extends ChangeIndexPredicate implements HasCardinality {
   protected final Account.Id accountId;
   protected final String label;
 
@@ -39,6 +40,11 @@ public class StarPredicate extends ChangeIndexPredicate {
   }
 
   @Override
+  public int getCardinality() {
+    return 10;
+  }
+
+  @Override
   public String toString() {
     return ChangeQueryBuilder.FIELD_STAR + ":" + label;
   }
diff --git a/java/com/google/gerrit/server/query/change/TrackingIdPredicate.java b/java/com/google/gerrit/server/query/change/TrackingIdPredicate.java
index 622fa2c4fc..0d1bd67e77 100644
--- a/java/com/google/gerrit/server/query/change/TrackingIdPredicate.java
+++ b/java/com/google/gerrit/server/query/change/TrackingIdPredicate.java
@@ -14,9 +14,10 @@
 
 package com.google.gerrit.server.query.change;
 
+import com.google.gerrit.index.query.HasCardinality;
 import com.google.gerrit.server.index.change.ChangeField;
 
-public class TrackingIdPredicate extends ChangeIndexPredicate {
+public class TrackingIdPredicate extends ChangeIndexPredicate implements HasCardinality {
   public TrackingIdPredicate(String trackingId) {
     super(ChangeField.TR, trackingId);
   }
@@ -30,4 +31,9 @@ public class TrackingIdPredicate extends ChangeIndexPredicate {
   public int getCost() {
     return 1;
   }
+
+  @Override
+  public int getCardinality() {
+    return 5;
+  }
 }