Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] IndicesQueryCache$CachingWeightWrapper#count should delegate to internal weight #10060

Closed
jainankitk opened this issue Sep 15, 2023 · 1 comment · Fixed by #10543
Closed
Assignees
Labels
bug Something isn't working good first issue Good for newcomers Search Search query, autocomplete ...etc

Comments

@jainankitk
Copy link
Collaborator

Describe the bug
Currently, IndicesQueryCache$CachingWeightWrapper#count does not delegate to internal weight object and keeps returning -1 from the Weight#count implementation. Due to this leaf counting cannot be done even when it is supported by Weight implementations like PointRangeQuery#createWeight

Expected behavior
Instrumenting the code for FiltersAggregator and observing with and without the change:

Without this change:

Original Query : class org.apache.lucene.document.LongPoint$1
Rewritten Query : class org.apache.lucene.search.IndexOrDocValuesQuery
Original Query : class org.apache.lucene.document.LongPoint$1
Rewritten Query : class org.apache.lucene.search.IndexOrDocValuesQuery
Wrapper class type : class org.apache.lucene.search.LRUQueryCache$CachingWrapperWeight
Aggregation count from the filters : 0  value : -1 class type : class org.apache.lucene.document.LongPoint$1 : class org.opensearch.indices.IndicesQueryCache$CachingWeightWrapper
Wrapper class type : class org.apache.lucene.search.LRUQueryCache$CachingWrapperWeight
Aggregation count from the filters : 1  value : -1 class type : class org.apache.lucene.document.LongPoint$1 : class org.opensearch.indices.IndicesQueryCache$CachingWeightWrapper

With the change:

Original Query : class org.apache.lucene.document.LongPoint$1
Rewritten Query : class org.apache.lucene.search.IndexOrDocValuesQuery
Original Query : class org.apache.lucene.document.LongPoint$1
Rewritten Query : class org.apache.lucene.search.IndexOrDocValuesQuery
Wrapper class type : class org.apache.lucene.search.LRUQueryCache$CachingWrapperWeight
PointRangeQuery#count method was invoked
Aggregation count from the filters : 0  value : 4349068 class type : class org.apache.lucene.document.LongPoint$1 : class org.opensearch.indices.IndicesQueryCache$CachingWeightWrapper
Wrapper class type : class org.apache.lucene.search.LRUQueryCache$CachingWrapperWeight
PointRangeQuery#count method was invoked
Aggregation count from the filters : 1  value : 5981766 class type : class org.apache.lucene.document.LongPoint$1 : class org.opensearch.indices.IndicesQueryCache$CachingWeightWrapper

Additional context
Instrumentation Code

diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregator.java
index 84e0218d8ea..b54b46a8254 100644
--- a/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregator.java
+++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregator.java
@@ -175,6 +175,7 @@ public class FiltersAggregator extends BucketsAggregator {
         Weight[] filters = this.filters.get();
         final Bits[] bits = new Bits[filters.length];
         for (int i = 0; i < filters.length; ++i) {
+            System.out.println("Aggregation count from the filters : " + i + "  value : " + filters[i].count(ctx) + " class type : " + filters[i].getQuery().getClass() + " : " + filters[i].getClass());
             bits[i] = Lucene.asSequentialAccessBits(ctx.reader().maxDoc(), filters[i].scorerSupplier(ctx));
         }
         return new LeafBucketCollectorBase(sub, null) {
diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java
index 8741213f988..be96c0505b2 100644
--- a/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java
+++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java
@@ -32,10 +32,7 @@
 
 package org.opensearch.search.aggregations.bucket.filter;
 
-import org.apache.lucene.search.IndexSearcher;
-import org.apache.lucene.search.Query;
-import org.apache.lucene.search.ScoreMode;
-import org.apache.lucene.search.Weight;
+import org.apache.lucene.search.*;
 import org.opensearch.index.query.QueryShardContext;
 import org.opensearch.search.aggregations.AggregationInitializationException;
 import org.opensearch.search.aggregations.Aggregator;
@@ -102,7 +99,9 @@ public class FiltersAggregatorFactory extends AggregatorFactory {
                 IndexSearcher contextSearcher = searchContext.searcher();
                 weights = new Weight[filters.length];
                 for (int i = 0; i < filters.length; ++i) {
-                    this.weights[i] = contextSearcher.createWeight(contextSearcher.rewrite(filters[i]), ScoreMode.COMPLETE_NO_SCORES, 1);
+                    System.out.println("Original Query : " + ((IndexOrDocValuesQuery)filters[i]).getIndexQuery().getClass());
+                    System.out.println("Rewritten Query : " + contextSearcher.rewrite(filters[i]).getClass());
+                    this.weights[i] = contextSearcher.createWeight(((IndexOrDocValuesQuery)filters[i]).getIndexQuery(), ScoreMode.COMPLETE_NO_SCORES, 1);
                 }
             } catch (IOException e) {
                 throw new AggregationInitializationException("Failed to initialse filters for aggregation [" + name() + "]", e);

Code change:

diff --git a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java
index 2669da3f417..97718aeca34 100644
--- a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java
+++ b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java
@@ -186,6 +186,12 @@ public class IndicesQueryCache implements QueryCache, Closeable {
             return in.bulkScorer(context);
         }
 
+        @Override
+        public int count(LeafReaderContext context) throws IOException {
+            System.out.println("Wrapper class type : " + in.getClass());
+            return in.count(context);
+        }
+
         @Override
         public boolean isCacheable(LeafReaderContext ctx) {
             return in.isCacheable(ctx);
@jainankitk jainankitk added bug Something isn't working untriaged labels Sep 15, 2023
@jainankitk
Copy link
Collaborator Author

Related to #9310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Search Search query, autocomplete ...etc
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants