From f7bdbe13dfff895a63dd337c21df276fdc53339a Mon Sep 17 00:00:00 2001 From: bowenlan-amzn Date: Mon, 29 Jan 2024 11:13:31 -0800 Subject: [PATCH] small refactor Signed-off-by: bowenlan-amzn --- .../bucket/FastFilterRewriteHelper.java | 47 ++++++++++++------- .../histogram/DateHistogramAggregator.java | 1 - 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java index 382914c496713..9b7a9fdfd5bde 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java @@ -13,6 +13,7 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.PointValues; import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.FieldExistsQuery; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.PointRangeQuery; @@ -122,6 +123,11 @@ public static long[] getAggregationBounds(final SearchContext context, final Str } } else if (cq instanceof MatchAllDocsQuery) { return indexBounds; + } else if (cq instanceof FieldExistsQuery) { + // when a range query covers all values of a shard, it will be rewrite field exists query + if (((FieldExistsQuery) cq).getField().equals(fieldName)) { + return indexBounds; + } } return null; @@ -187,14 +193,12 @@ protected String toString(int dimension, byte[] value) { /** * Context object for fast filter optimization - * - * filters equals to null if the optimization cannot be applied */ public static class FastFilterContext { private boolean rewriteable = false; private Weight[] filters = null; public AggregationType aggregationType; - protected final SearchContext context; + private final SearchContext context; public FastFilterContext(SearchContext context) { this.context = context; @@ -217,14 +221,14 @@ public void buildFastFilter() throws IOException { this.filters = filters; } - public Weight[] buildFastFilter(LeafReaderContext ctx) throws IOException { + private Weight[] buildFastFilter(LeafReaderContext ctx) throws IOException { Weight[] filters = this.aggregationType.buildFastFilter(context, ctx); logger.debug("Fast filter built: {}", filters == null ? "no" : "yes"); this.filters = filters; return filters; } - public boolean hasfilterBuilt() { + public boolean filterBuilt() { return filters != null; } } @@ -236,7 +240,6 @@ public interface AggregationType { boolean isRewriteable(Object parent, int subAggLength); - // Weight[] buildFastFilter(SearchContext context) throws IOException; Weight[] buildFastFilter(SearchContext context, LeafReaderContext ctx) throws IOException; } @@ -347,17 +350,20 @@ public static boolean tryFastFilterAggregation( final BiConsumer incrementDocCount ) throws IOException { if (fastFilterContext == null) return false; - if (!fastFilterContext.hasfilterBuilt() && fastFilterContext.rewriteable) { - // Check if the query is functionally match-all at segment level - // if so, we still compute the index bounds and use it to build the filters - SearchContext context = fastFilterContext.context; - Weight weight = context.searcher().createWeight(context.query(), ScoreMode.COMPLETE_NO_SCORES, 1f); - boolean segmentRewriteable = weight != null && weight.count(ctx) == ctx.reader().numDocs(); - if (segmentRewriteable) { - fastFilterContext.buildFastFilter(ctx); + + // check if the query is functionally match-all at segment level + if (segmentMatchAll(fastFilterContext.context, ctx)) { + logger.debug("Functionally match all for the segment {}", ctx); + if (fastFilterContext.rewriteable) { + if (!fastFilterContext.filterBuilt()) { + fastFilterContext.buildFastFilter(ctx); + } + } else { + return false; } } - if (!fastFilterContext.hasfilterBuilt()) return false; + + if (!fastFilterContext.filterBuilt()) return false; final Weight[] filters = fastFilterContext.filters; final int[] counts = new int[filters.length]; @@ -388,10 +394,19 @@ public static boolean tryFastFilterAggregation( } incrementDocCount.accept(bucketKey, counts[i]); s++; - if (s > size) return true; + if (s > size) { + logger.debug("Fast filter optimization applied with size {}", size); + return true; + } } } + logger.debug("Fast filter optimization applied"); return true; } + + private static boolean segmentMatchAll(SearchContext ctx, LeafReaderContext leafCtx) throws IOException { + Weight weight = ctx.searcher().createWeight(ctx.query(), ScoreMode.COMPLETE_NO_SCORES, 1f); + return weight != null && weight.count(leafCtx) == leafCtx.reader().numDocs(); + } } diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java index 5b896cb95cf25..db257e85aefd1 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java @@ -175,7 +175,6 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucketCol count ) ); - logger.info("optimized = " + optimized); if (optimized) throw new CollectionTerminatedException(); SortedNumericDocValues values = valuesSource.longValues(ctx);