Cleaning

Signed-off-by: bowenlan-amzn <[email protected]>
bowenlan-amzn · Dec 6, 2023 · d1674eb · d1674eb
1 parent 157b2e5
commit d1674eb
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 30 deletions.
diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/BucketsAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/BucketsAggregator.java
@@ -422,10 +422,7 @@ protected final <B> InternalAggregation[] buildAggregationsForVariableBuckets(
                             + "]"
                     );
                 }
-                buckets.add(bucketBuilder.build(
-                    ordsEnum.value(),
-                    bucketDocCount(ordsEnum.ord()),
-                    subAggregationResults[b++]));
+                buckets.add(bucketBuilder.build(ordsEnum.value(), bucketDocCount(ordsEnum.ord()), subAggregationResults[b++]));
             }
             results[ordIdx] = resultBuilder.build(owningBucketOrds[ordIdx], buckets);
         }

diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/FilterRewriteHelper.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/FilterRewriteHelper.java
@@ -146,7 +146,7 @@ private static Weight[] createFilterForAggregations(
             // Below rounding is needed as the interval could return in
             // non-rounded values for something like calendar month
             roundedLow = preparedRounding.round(roundedLow + interval);
-            if (prevRounded == roundedLow) break; // TODO reading prevents getting into an infinite loop?
+            if (prevRounded == roundedLow) break; // prevents getting into an infinite loop
             prevRounded = roundedLow;
         }
 
@@ -210,9 +210,9 @@ public static FilterContext buildFastFilterContext(
         CheckedFunction<ValueSourceContext, long[], IOException> computeBounds
     ) throws IOException {
         if (parent == null && subAggLength == 0 && !valueSourceContext.missing && !valueSourceContext.hasScript) {
-            MappedFieldType fieldType = valueSourceContext.getFieldType();
+            MappedFieldType fieldType = valueSourceContext.fieldType;
             if (fieldType != null) {
-                final String fieldName = valueSourceContext.getFieldType().name();
+                final String fieldName = fieldType.name();
                 final long[] bounds = computeBounds.apply(valueSourceContext);
                 if (bounds != null) {
                     assert fieldType instanceof DateFieldMapper.DateFieldType;
@@ -252,7 +252,6 @@ public ValueSourceContext(boolean missing, boolean hasScript, MappedFieldType fi
             this.fieldType = fieldType;
         }
 
-        // TODO reading why boolean doesn't need getter?
         public MappedFieldType getFieldType() {
             return fieldType;
         }

diff --git a/...rc/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java b/...rc/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java
@@ -63,7 +63,15 @@
 import org.opensearch.index.mapper.DateFieldMapper;
 import org.opensearch.lucene.queries.SearchAfterSortedDocQuery;
 import org.opensearch.search.DocValueFormat;
-import org.opensearch.search.aggregations.*;
+import org.opensearch.search.aggregations.Aggregator;
+import org.opensearch.search.aggregations.AggregatorFactories;
+import org.opensearch.search.aggregations.BucketCollector;
+import org.opensearch.search.aggregations.CardinalityUpperBound;
+import org.opensearch.search.aggregations.InternalAggregation;
+import org.opensearch.search.aggregations.InternalAggregations;
+import org.opensearch.search.aggregations.LeafBucketCollector;
+import org.opensearch.search.aggregations.MultiBucketCollector;
+import org.opensearch.search.aggregations.MultiBucketConsumerService;
 import org.opensearch.search.aggregations.bucket.BucketsAggregator;
 import org.opensearch.search.aggregations.bucket.missing.MissingOrder;
 import org.opensearch.search.aggregations.bucket.terms.LongKeyedBucketOrds;

diff --git a/...va/org/opensearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueue.java b/...va/org/opensearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueue.java
@@ -108,7 +108,7 @@ public int hashCode() {
 
     @Override
     protected boolean lessThan(Integer a, Integer b) {
-        return compare(a, b) > 0; // TODO reading a > b is true, this is a max heap?
+        return compare(a, b) > 0; // max heap
     }
 
     /**
@@ -123,7 +123,7 @@ boolean isFull() {
      * the slot if the candidate is already in the queue or null if the candidate is not present.
      */
     Integer compareCurrent() {
-        return map.get(new Slot(CANDIDATE_SLOT)); // TODO reading this check the slot/bucket? of the current value
+        return map.get(new Slot(CANDIDATE_SLOT));
     }
 
     /**
@@ -152,7 +152,7 @@ long getDocCount(int slot) {
      */
     private void copyCurrent(int slot, long value) {
         for (int i = 0; i < arrays.length; i++) {
-            arrays[i].copyCurrent(slot); // TODO reading valueSource knows current value, set the value to this slot/index
+            arrays[i].copyCurrent(slot);
         }
         docCounts = bigArrays.grow(docCounts, slot + 1);
         docCounts.set(slot, value);
@@ -204,7 +204,7 @@ boolean equals(int slot1, int slot2) {
     int hashCode(int slot) {
         int result = 1;
         for (int i = 0; i < arrays.length; i++) {
-            result = 31 * result + // TODO reading why 31 here? For each array, it multiplies the running result by 31. Multiplying by a prime number like 31 helps distribute the hash codes more evenly.
+            result = 31 * result +
                 (slot == CANDIDATE_SLOT ?
                     arrays[i].hashCodeCurrent() :
                     arrays[i].hashCode(slot));
@@ -256,7 +256,7 @@ LeafBucketCollector getLeafCollector(Comparable forceLeadSourceValue, LeafReader
         int last = arrays.length - 1;
         LeafBucketCollector collector = in;
         while (last > 0) {
-            collector = arrays[last--].getLeafCollector(context, collector); // TODO reading the pass-in collect will work after current
+            collector = arrays[last--].getLeafCollector(context, collector);
         }
 
         if (forceLeadSourceValue != null) {

diff --git a/...r/src/main/java/org/opensearch/search/aggregations/bucket/composite/LongValuesSource.java b/...r/src/main/java/org/opensearch/search/aggregations/bucket/composite/LongValuesSource.java
@@ -253,8 +253,7 @@ static boolean checkMatchAllOrRangeQuery(Query query, String fieldName) {
     @Override
     SortedDocsProducer createSortedDocsProducerOrNull(IndexReader reader, Query query) {
         query = extractQuery(query);
-        if (checkIfSortedDocsIsApplicable(reader, fieldType) == false
-            || checkMatchAllOrRangeQuery(query, fieldType.name()) == false) {
+        if (checkIfSortedDocsIsApplicable(reader, fieldType) == false || checkMatchAllOrRangeQuery(query, fieldType.name()) == false) {
             return null;
         }
         final byte[] lowerPoint;

diff --git a/...in/java/org/opensearch/search/aggregations/bucket/composite/PointsSortedDocsProducer.java b/...in/java/org/opensearch/search/aggregations/bucket/composite/PointsSortedDocsProducer.java
@@ -148,8 +148,9 @@ public void visit(int docID, byte[] packedValue) throws IOException {
             }
 
             long bucket = bucketFunction.applyAsLong(packedValue);
-            if (first == false && bucket != lastBucket) { // TODO reading process previous bucket when new bucket appears
-                if (processBucket(queue, context, bucketDocsBuilder.build().iterator(), lastBucket, builder) &&
+            if (first == false && bucket != lastBucket) { // process previous bucket when new bucket appears
+                final DocIdSet docIdSet = bucketDocsBuilder.build();
+                if (processBucket(queue, context, docIdSet.iterator(), lastBucket, builder) &&
                     // lower bucket is inclusive
                     lowerBucket != lastBucket) {
                     // this bucket does not have any competitive composite buckets,

diff --git a/...java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java b/...java/org/opensearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java
@@ -185,8 +185,7 @@ protected boolean checkIfSortedDocsIsApplicable(IndexReader reader, MappedFieldT
             return false;
         }
 
-        if (reader.hasDeletions()
-            && (reader.numDocs() == 0 || (double) reader.numDocs() / (double) reader.maxDoc() < 0.5)) {
+        if (reader.hasDeletions() && (reader.numDocs() == 0 || (double) reader.numDocs() / (double) reader.maxDoc() < 0.5)) {
             // do not use the index if it has more than 50% of deleted docs
             return false;
         }

diff --git a/...src/main/java/org/opensearch/search/aggregations/bucket/composite/SortedDocsProducer.java b/...src/main/java/org/opensearch/search/aggregations/bucket/composite/SortedDocsProducer.java
@@ -91,7 +91,7 @@ protected boolean processBucket(
             @Override
             public void collect(int doc, long bucket) throws IOException {
                 hasCollected[0] = true;
-                long docCount = docCountProvider.getDocCount(doc); // TODO reading _doc_count can be >1
+                long docCount = docCountProvider.getDocCount(doc);
                 if (queue.addIfCompetitive(docCount)) {
                     topCompositeCollected[0]++;
                     if (adder != null && doc != lastDoc) { // TODO reading why doc can be == lastDoc?
@@ -109,11 +109,10 @@ public void collect(int doc, long bucket) throws IOException {
             }
         };
 
-        final LeafBucketCollector collector = queue.getLeafCollector(leadSourceBucket, context, queueCollector);
-
         final Bits liveDocs = context.reader().getLiveDocs();
+        final LeafBucketCollector collector = queue.getLeafCollector(leadSourceBucket, context, queueCollector);
         while (iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            if (liveDocs == null || liveDocs.get(iterator.docID())) { // TODO reading doc exists
+            if (liveDocs == null || liveDocs.get(iterator.docID())) {
                 collector.collect(iterator.docID());
             }
         }

diff --git a/...org/opensearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java b/...org/opensearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java
@@ -441,12 +441,10 @@ public void testUnmappedMissing() throws IOException {
 
         final DateFieldMapper.DateFieldType fieldType = new DateFieldMapper.DateFieldType("date_field");
 
-        testCase(aggregation, DEFAULT_QUERY, iw -> {},
-            (Consumer<InternalAutoDateHistogram>) histogram -> {
-                assertEquals(0, histogram.getBuckets().size());
-                assertFalse(AggregationInspectionHelper.hasValue(histogram));
-            },
-            fieldType);
+        testCase(aggregation, DEFAULT_QUERY, iw -> {}, (Consumer<InternalAutoDateHistogram>) histogram -> {
+            assertEquals(0, histogram.getBuckets().size());
+            assertFalse(AggregationInspectionHelper.hasValue(histogram));
+        }, fieldType);
     }
 
     public void testIntervalYear() throws IOException {