From 49e1e25fb70850dd88b056c43e72ec9af73194c3 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Wed, 26 Mar 2025 12:58:31 -0500 Subject: [PATCH 01/12] added singleton optimization path to globalordinalvaluesource Signed-off-by: shreyah963 --- .../composite/GlobalOrdinalValuesSource.java | 67 ++++++++++++++++--- .../SingleDimensionValuesSource.java | 24 +++++++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index 227dce543cfe9..9bed069a6f00d 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -32,9 +32,7 @@ package org.opensearch.search.aggregations.bucket.composite; -import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.*; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; @@ -81,6 +79,8 @@ class GlobalOrdinalValuesSource extends SingleDimensionValuesSource { super(bigArrays, format, type, missingBucket, missingOrder, size, reverseMul); this.docValuesFunc = docValuesFunc; this.values = bigArrays.newLongArray(Math.min(size, 100), false); + this.isSingleValued = false; + this.singletonValues = null; } @Override @@ -167,21 +167,72 @@ BytesRef toComparable(int slot) throws IOException { @Override LeafBucketCollector getLeafCollector(LeafReaderContext context, LeafBucketCollector next) throws IOException { + // Get the DocValues for this segment/leaf of the index final SortedSetDocValues dvs = docValuesFunc.apply(context); + + // Initialize lookup table for ordinals if not already done if (lookup == null) { initLookup(dvs); } + + // Try to optimize by converting multi-valued field to single-valued + SortedDocValues sorted = DocValues.unwrapSingleton(dvs); + + // Field is single-valued if: + // 1. Successfully unwrapped to SortedDocValues, or + // 2. All docs have exactly one value + isSingleValued = sorted != null || dvs.docValueCount() == 1; + + // Store the optimized values: + // - If unwrap succeeded, use unwrapped version + // - If single-valued but unwrap failed, use original + // - If multi-valued, use null + singletonValues = sorted != null ? sorted : (isSingleValued ? dvs : null); + + if (isSingleValued) { + // Optimized collector for single-valued fields + return new LeafBucketCollector() { + @Override + public void collect(int doc, long bucket) throws IOException { + if (singletonValues instanceof SortedDocValues) { + // Handle native single-value format + SortedDocValues values = (SortedDocValues) singletonValues; + if (values.advanceExact(doc)) { // If document has a value + currentValue = values.ordValue(); // Get ordinal directly + next.collect(doc, bucket); // Collect into bucket + } else if (missingBucket) { // Handle missing value case + currentValue = -1; // Use -1 for missing + next.collect(doc, bucket); + } + } else { + // Handle SortedSetDocValues that we know contains single values + SortedSetDocValues values = (SortedSetDocValues) singletonValues; + if (values.advanceExact(doc) && // If doc has values and + (values.nextOrd() != SortedSetDocValues.NO_MORE_DOCS)) { // has valid ordinal + currentValue = values.nextOrd(); // Get the ordinal + next.collect(doc, bucket); + } else if (missingBucket) { // Handle missing/empty cases + currentValue = -1; + next.collect(doc, bucket); + } + } + } + }; + } + + // Non-optimized collector for multi-valued fields return new LeafBucketCollector() { @Override public void collect(int doc, long bucket) throws IOException { - if (dvs.advanceExact(doc)) { + if (dvs.advanceExact(doc)) { // If document has values long ord; - int count = dvs.docValueCount(); + int count = dvs.docValueCount(); // Get number of values + // Loop through all values in the document while ((count-- > 0) && (ord = dvs.nextOrd()) != NO_MORE_DOCS) { - currentValue = ord; - next.collect(doc, bucket); + currentValue = ord; // Store current ordinal + next.collect(doc, bucket); // Collect each value } - } else if (missingBucket) { + } else if (missingBucket) { // Handle missing values currentValue = -1; next.collect(doc, bucket); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java index fe0801d6d230e..3949b6a19e904 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java @@ -32,8 +32,10 @@ package org.opensearch.search.aggregations.bucket.composite; +import org.apache.lucene.index.DocValues; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.search.Query; import org.opensearch.common.Nullable; import org.opensearch.common.lease.Releasable; @@ -66,6 +68,9 @@ public abstract class SingleDimensionValuesSource> imple protected T afterValue; + protected boolean isSingleValued; + protected Object singletonValues; // Generic type to support different DocValues types + /** * Creates a new {@link SingleDimensionValuesSource}. * @@ -94,6 +99,25 @@ public abstract class SingleDimensionValuesSource> imple this.size = size; this.reverseMul = reverseMul; this.afterValue = null; + + // Initialize singleton optimization fields + this.isSingleValued = false; + this.singletonValues = null; + } + + /** + * Returns the singleton values if available + */ + protected Object getSingletonValues() { + return singletonValues; + } + + /** + * Sets the singleton optimization state + */ + protected void setSingletonOptimization(boolean isSingleValued, Object values) { + this.isSingleValued = isSingleValued; + this.singletonValues = values; } /** From 5b312326179dad63701a179b5124c7b0bced8aac Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Wed, 26 Mar 2025 13:23:11 -0500 Subject: [PATCH 02/12] enabled remote debugging Signed-off-by: shreyah963 --- distribution/src/config/jvm.options | 2 ++ 1 file changed, 2 insertions(+) diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index a8c96f33ce51d..364bd2246516b 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -89,3 +89,5 @@ ${error.file} # See please https://bugs.openjdk.org/browse/JDK-8341127 (openjdk/jdk#21283) 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.setAsTypeCache 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.asTypeUncached + +-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 From 27d1170278b74be5696346f342b72886f167f13d Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Thu, 27 Mar 2025 12:52:33 -0500 Subject: [PATCH 03/12] Removed the minimum cap to handle larger ordinal values Signed-off-by: shreyah963 --- .../bucket/composite/GlobalOrdinalValuesSource.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index 9bed069a6f00d..f7efc36ed23ba 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -78,7 +78,7 @@ class GlobalOrdinalValuesSource extends SingleDimensionValuesSource { ) { super(bigArrays, format, type, missingBucket, missingOrder, size, reverseMul); this.docValuesFunc = docValuesFunc; - this.values = bigArrays.newLongArray(Math.min(size, 100), false); + this.values = bigArrays.newLongArray(size, false); this.isSingleValued = false; this.singletonValues = null; } @@ -178,7 +178,7 @@ LeafBucketCollector getLeafCollector(LeafReaderContext context, LeafBucketCollec // Try to optimize by converting multi-valued field to single-valued SortedDocValues sorted = DocValues.unwrapSingleton(dvs); - // Field is single-valued if: + //; Field is single-valued if: // 1. Successfully unwrapped to SortedDocValues, or // 2. All docs have exactly one value isSingleValued = sorted != null || dvs.docValueCount() == 1; From 6f28b4022dcc7d4d5eeb1067c086e39adc38ab0a Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Fri, 28 Mar 2025 11:09:24 -0500 Subject: [PATCH 04/12] emove redundant singleton optimization state from SingleDimensionValuesSource and simplify the optimization logic in GlobalOrdinalValuesSource. The singleton optimization is now only applied when DocValues.unwrapSingleton() succeeds, preventing array index out of bounds errors with high cardinality fields. Signed-off-by: shreyah963 --- .../composite/GlobalOrdinalValuesSource.java | 44 ++++--------------- .../SingleDimensionValuesSource.java | 22 ---------- 2 files changed, 8 insertions(+), 58 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index f7efc36ed23ba..096b602e22385 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -175,46 +175,18 @@ LeafBucketCollector getLeafCollector(LeafReaderContext context, LeafBucketCollec initLookup(dvs); } - // Try to optimize by converting multi-valued field to single-valued - SortedDocValues sorted = DocValues.unwrapSingleton(dvs); + final SortedDocValues singleton = DocValues.unwrapSingleton(dvs); - //; Field is single-valued if: - // 1. Successfully unwrapped to SortedDocValues, or - // 2. All docs have exactly one value - isSingleValued = sorted != null || dvs.docValueCount() == 1; - - // Store the optimized values: - // - If unwrap succeeded, use unwrapped version - // - If single-valued but unwrap failed, use original - // - If multi-valued, use null - singletonValues = sorted != null ? sorted : (isSingleValued ? dvs : null); - - if (isSingleValued) { - // Optimized collector for single-valued fields + if (singleton != null) { return new LeafBucketCollector() { @Override public void collect(int doc, long bucket) throws IOException { - if (singletonValues instanceof SortedDocValues) { - // Handle native single-value format - SortedDocValues values = (SortedDocValues) singletonValues; - if (values.advanceExact(doc)) { // If document has a value - currentValue = values.ordValue(); // Get ordinal directly - next.collect(doc, bucket); // Collect into bucket - } else if (missingBucket) { // Handle missing value case - currentValue = -1; // Use -1 for missing - next.collect(doc, bucket); - } - } else { - // Handle SortedSetDocValues that we know contains single values - SortedSetDocValues values = (SortedSetDocValues) singletonValues; - if (values.advanceExact(doc) && // If doc has values and - (values.nextOrd() != SortedSetDocValues.NO_MORE_DOCS)) { // has valid ordinal - currentValue = values.nextOrd(); // Get the ordinal - next.collect(doc, bucket); - } else if (missingBucket) { // Handle missing/empty cases - currentValue = -1; - next.collect(doc, bucket); - } + if (singleton.advanceExact(doc)) { // If document has a value + currentValue = singleton.ordValue(); // Get ordinal directly + next.collect(doc, bucket); // Collect into bucket + } else if (missingBucket) { // Handle missing value case + currentValue = -1; // Use -1 for missing + next.collect(doc, bucket); } } }; diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java index 3949b6a19e904..e960000869eab 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java @@ -68,9 +68,6 @@ public abstract class SingleDimensionValuesSource> imple protected T afterValue; - protected boolean isSingleValued; - protected Object singletonValues; // Generic type to support different DocValues types - /** * Creates a new {@link SingleDimensionValuesSource}. * @@ -99,25 +96,6 @@ public abstract class SingleDimensionValuesSource> imple this.size = size; this.reverseMul = reverseMul; this.afterValue = null; - - // Initialize singleton optimization fields - this.isSingleValued = false; - this.singletonValues = null; - } - - /** - * Returns the singleton values if available - */ - protected Object getSingletonValues() { - return singletonValues; - } - - /** - * Sets the singleton optimization state - */ - protected void setSingletonOptimization(boolean isSingleValued, Object values) { - this.isSingleValued = isSingleValued; - this.singletonValues = values; } /** From 8c61203b5db675d249302bf979917c1c5dbfb534 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Fri, 28 Mar 2025 11:14:57 -0500 Subject: [PATCH 05/12] removed redundant initialization Signed-off-by: shreyah963 --- .../bucket/composite/GlobalOrdinalValuesSource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index 096b602e22385..03cc95e7c6c9d 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -79,8 +79,6 @@ class GlobalOrdinalValuesSource extends SingleDimensionValuesSource { super(bigArrays, format, type, missingBucket, missingOrder, size, reverseMul); this.docValuesFunc = docValuesFunc; this.values = bigArrays.newLongArray(size, false); - this.isSingleValued = false; - this.singletonValues = null; } @Override From c2ffe85bbfa456901b0ae7b2ebfc14ac886c55ab Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Fri, 28 Mar 2025 11:17:16 -0500 Subject: [PATCH 06/12] reverted the array allocation in the constructer to its original form Signed-off-by: shreyah963 --- .../bucket/composite/GlobalOrdinalValuesSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index 03cc95e7c6c9d..e68375e2c9a0f 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -78,7 +78,7 @@ class GlobalOrdinalValuesSource extends SingleDimensionValuesSource { ) { super(bigArrays, format, type, missingBucket, missingOrder, size, reverseMul); this.docValuesFunc = docValuesFunc; - this.values = bigArrays.newLongArray(size, false); + this.values = bigArrays.newLongArray(Math.min(size, 100), false); } @Override From 546605758239b9a61566a716cb885d5d39d4dfc3 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Mon, 31 Mar 2025 18:08:26 -0500 Subject: [PATCH 07/12] [Docs] Add detailed comments to GlobalOrdinalValuesSource collector Signed-off-by: shreyah963 --- .../bucket/composite/GlobalOrdinalValuesSource.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index e68375e2c9a0f..3886dc649526a 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -165,16 +165,15 @@ BytesRef toComparable(int slot) throws IOException { @Override LeafBucketCollector getLeafCollector(LeafReaderContext context, LeafBucketCollector next) throws IOException { - // Get the DocValues for this segment/leaf of the index final SortedSetDocValues dvs = docValuesFunc.apply(context); - - // Initialize lookup table for ordinals if not already done if (lookup == null) { initLookup(dvs); } + // unwrapSingleton() returns non-null only if the field is single-valued final SortedDocValues singleton = DocValues.unwrapSingleton(dvs); + // Optimization path: Field is confirmed to be single-valued if (singleton != null) { return new LeafBucketCollector() { @Override From c9639aea20dfd67bfc73c6f5ef47b554c38ac91e Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Mon, 31 Mar 2025 18:12:18 -0500 Subject: [PATCH 08/12] Remote redundant imports and disable remote debugging Signed-off-by: shreyah963 --- distribution/src/config/jvm.options | 2 -- .../bucket/composite/SingleDimensionValuesSource.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index 364bd2246516b..a8c96f33ce51d 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -89,5 +89,3 @@ ${error.file} # See please https://bugs.openjdk.org/browse/JDK-8341127 (openjdk/jdk#21283) 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.setAsTypeCache 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.asTypeUncached - --agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java index e960000869eab..fe0801d6d230e 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java @@ -32,10 +32,8 @@ package org.opensearch.search.aggregations.bucket.composite; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.search.Query; import org.opensearch.common.Nullable; import org.opensearch.common.lease.Releasable; From 4a13f79ba905822f9428e8acb4c486a773708c03 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Tue, 1 Apr 2025 14:29:23 -0500 Subject: [PATCH 09/12] replaced wildcard import with only necessary imports Signed-off-by: shreyah963 --- .../bucket/composite/GlobalOrdinalValuesSource.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index 3886dc649526a..e5d175d4c03f7 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -32,7 +32,11 @@ package org.opensearch.search.aggregations.bucket.composite; -import org.apache.lucene.index.*; +import org.apache.lucene.index.DocValues; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; From b50f38ccef5b99d12a7c66c9bfdcbccca1d64238 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Tue, 8 Apr 2025 08:34:14 -0700 Subject: [PATCH 10/12] Update CHANGELOG.md Signed-off-by: shreyah963 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60820ab196261..112304414a805 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Security Manager Replacement] Enhance Java Agent to intercept Runtime::halt ([#17757](https://github.com/opensearch-project/OpenSearch/pull/17757)) - Support AutoExpand for SearchReplica ([#17741](https://github.com/opensearch-project/OpenSearch/pull/17741)) - Implement fixed interval refresh task scheduling ([#17777](https://github.com/opensearch-project/OpenSearch/pull/17777)) +- ### Changed - Migrate BC libs to their FIPS counterparts ([#14912](https://github.com/opensearch-project/OpenSearch/pull/14912)) - Increase the floor segment size to 16MB ([#17699](https://github.com/opensearch-project/OpenSearch/pull/17699)) +- Optimize composite aggregation performance for single-valued fields by eliminating iterative ordinal processing ([#17740](https://github.com/opensearch-project/OpenSearch/pull/17740)) ### Dependencies - Bump `com.nimbusds:nimbus-jose-jwt` from 9.41.1 to 10.0.2 ([#17607](https://github.com/opensearch-project/OpenSearch/pull/17607), [#17669](https://github.com/opensearch-project/OpenSearch/pull/17669)) From c22e32a8b01964cfb37fcd992d228617e41576b8 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Tue, 8 Apr 2025 09:43:51 -0700 Subject: [PATCH 11/12] Update CHANGELOG.md Co-authored-by: bowenlan-amzn Signed-off-by: shreyah963 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c70389438df3d..8661cb7d70942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Migrate BC libs to their FIPS counterparts ([#14912](https://github.com/opensearch-project/OpenSearch/pull/14912)) - Increase the floor segment size to 16MB ([#17699](https://github.com/opensearch-project/OpenSearch/pull/17699)) -- Optimize composite aggregation performance for single-valued fields by eliminating iterative ordinal processing ([#17740](https://github.com/opensearch-project/OpenSearch/pull/17740)) +- Unwrap singleton DocValues in global ordinal value source of composite histogram aggregation ([#17740](https://github.com/opensearch-project/OpenSearch/pull/17740)) - Introduce 512 byte limit to search and ingest pipeline IDs ([#17786](https://github.com/opensearch-project/OpenSearch/pull/17786)) ### Dependencies From d93f5898daceb2f7163a7ddbe76476e2ef5e98e3 Mon Sep 17 00:00:00 2001 From: shreyah963 Date: Tue, 8 Apr 2025 12:03:53 -0500 Subject: [PATCH 12/12] Remove redundant comments from GlobalOrdinalValuesSource Signed-off-by: shreyah963 --- .../composite/GlobalOrdinalValuesSource.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java index e5d175d4c03f7..ad1116d842360 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/GlobalOrdinalValuesSource.java @@ -177,35 +177,33 @@ LeafBucketCollector getLeafCollector(LeafReaderContext context, LeafBucketCollec // unwrapSingleton() returns non-null only if the field is single-valued final SortedDocValues singleton = DocValues.unwrapSingleton(dvs); - // Optimization path: Field is confirmed to be single-valued + // Direct ordinal access for single-valued fields if (singleton != null) { return new LeafBucketCollector() { @Override public void collect(int doc, long bucket) throws IOException { - if (singleton.advanceExact(doc)) { // If document has a value - currentValue = singleton.ordValue(); // Get ordinal directly - next.collect(doc, bucket); // Collect into bucket - } else if (missingBucket) { // Handle missing value case - currentValue = -1; // Use -1 for missing + if (singleton.advanceExact(doc)) { + currentValue = singleton.ordValue(); + next.collect(doc, bucket); + } else if (missingBucket) { + currentValue = -1; next.collect(doc, bucket); } } }; } - // Non-optimized collector for multi-valued fields return new LeafBucketCollector() { @Override public void collect(int doc, long bucket) throws IOException { - if (dvs.advanceExact(doc)) { // If document has values + if (dvs.advanceExact(doc)) { long ord; - int count = dvs.docValueCount(); // Get number of values - // Loop through all values in the document + int count = dvs.docValueCount(); while ((count-- > 0) && (ord = dvs.nextOrd()) != NO_MORE_DOCS) { - currentValue = ord; // Store current ordinal - next.collect(doc, bucket); // Collect each value + currentValue = ord; + next.collect(doc, bucket); } - } else if (missingBucket) { // Handle missing values + } else if (missingBucket) { currentValue = -1; next.collect(doc, bucket); }