Skip to content

Commit 03dc1e3

Browse files
sarthakaggarwal97sarthakaggarwal97
committed
nit fixes
Signed-off-by: Sarthak Aggarwal <[email protected]>
1 parent 0344858 commit 03dc1e3

File tree

9 files changed

+123
-16
lines changed

9 files changed

+123
-16
lines changed

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,17 @@ class CountValueAggregator implements ValueAggregator<Long> {
1818

1919
public static final long DEFAULT_INITIAL_VALUE = 1L;
2020
private final StarTreeNumericType starTreeNumericType;
21+
private final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.LONG;
2122

2223
public CountValueAggregator(StarTreeNumericType starTreeNumericType) {
2324
this.starTreeNumericType = starTreeNumericType;
2425
}
2526

27+
@Override
28+
public StarTreeNumericType getAggregatedValueType() {
29+
return VALUE_AGGREGATOR_TYPE;
30+
}
31+
2632
@Override
2733
public Long getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
2834

@@ -35,10 +41,11 @@ public Long getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
3541

3642
@Override
3743
public Long mergeAggregatedValueAndSegmentValue(Long value, Long segmentDocValue) {
38-
if (value == null) {
39-
return getIdentityMetricValue();
44+
assert value != null;
45+
if (segmentDocValue != null) {
46+
return value + 1;
4047
}
41-
return value + 1;
48+
return value;
4249
}
4350

4451
@Override

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,18 @@ abstract class StatelessDoubleValueAggregator implements ValueAggregator<Double>
1919

2020
protected final StarTreeNumericType starTreeNumericType;
2121
protected final Double identityValue;
22+
private final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.DOUBLE;
2223

2324
public StatelessDoubleValueAggregator(StarTreeNumericType starTreeNumericType, Double identityValue) {
2425
this.starTreeNumericType = starTreeNumericType;
2526
this.identityValue = identityValue;
2627
}
2728

29+
@Override
30+
public StarTreeNumericType getAggregatedValueType() {
31+
return VALUE_AGGREGATOR_TYPE;
32+
}
33+
2834
@Override
2935
public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
3036
if (segmentDocValue == null) {

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,19 @@
2323
class SumValueAggregator implements ValueAggregator<Double> {
2424

2525
private final StarTreeNumericType starTreeNumericType;
26+
private final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.DOUBLE;
2627

2728
private CompensatedSum kahanSummation = new CompensatedSum(0, 0);
2829

2930
public SumValueAggregator(StarTreeNumericType starTreeNumericType) {
3031
this.starTreeNumericType = starTreeNumericType;
3132
}
3233

34+
@Override
35+
public StarTreeNumericType getAggregatedValueType() {
36+
return VALUE_AGGREGATOR_TYPE;
37+
}
38+
3339
@Override
3440
public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
3541
kahanSummation.reset(0, 0);

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,20 @@
77
*/
88
package org.opensearch.index.compositeindex.datacube.startree.aggregators;
99

10+
import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
11+
1012
/**
1113
* A value aggregator that pre-aggregates on the input values for a specific type of aggregation.
1214
*
1315
* @opensearch.experimental
1416
*/
1517
public interface ValueAggregator<A> {
1618

19+
/**
20+
* Returns the data type of the aggregated value.
21+
*/
22+
StarTreeNumericType getAggregatedValueType();
23+
1724
/**
1825
* Returns the initial aggregated value.
1926
*/

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractDocumentsFileManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ protected long readMetrics(RandomAccessInput input, long offset, int numMetrics,
179179
input,
180180
offset,
181181
metrics,
182-
index -> metricAggregatorInfos.get(index).getValueAggregators().getIdentityMetricValue()
182+
index -> null
183183
);
184184
return offset;
185185
}

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ private Object[] getStarTreeMetricsFromSegment(int currentDocId, List<Sequential
303303
* @param segmentDocument segment star-tree document
304304
* @return merged star-tree document
305305
*/
306-
@SuppressWarnings({ "unchecked", "rawtypes" })
306+
@SuppressWarnings({"unchecked", "rawtypes"})
307307
protected StarTreeDocument reduceSegmentStarTreeDocuments(
308308
StarTreeDocument aggregatedSegmentDocument,
309309
StarTreeDocument segmentDocument,
@@ -470,17 +470,18 @@ public List<SequentialDocValuesIterator> getMetricReaders(SegmentWriteState stat
470470
List<SequentialDocValuesIterator> metricReaders = new ArrayList<>();
471471
for (Metric metric : this.starTreeField.getMetrics()) {
472472
for (MetricStat metricStat : metric.getMetrics()) {
473+
SequentialDocValuesIterator metricReader;
473474
FieldInfo metricFieldInfo = state.fieldInfos.fieldInfo(metric.getField());
474475
if (metricFieldInfo == null) {
475476
metricFieldInfo = getFieldInfo(metric.getField());
476477
}
477-
// TODO
478-
// if (metricStat != MetricStat.COUNT) {
479-
// Need not initialize the metric reader for COUNT metric type
480-
SequentialDocValuesIterator metricReader = new SequentialDocValuesIterator(
481-
fieldProducerMap.get(metricFieldInfo.name).getSortedNumeric(metricFieldInfo)
482-
);
483-
// }
478+
if (metricStat != MetricStat.COUNT) {
479+
metricReader = new SequentialDocValuesIterator(
480+
fieldProducerMap.get(metricFieldInfo.name).getSortedNumeric(metricFieldInfo)
481+
);
482+
} else {
483+
metricReader = new SequentialDocValuesIterator();
484+
}
484485

485486
metricReaders.add(metricReader);
486487
}

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.opensearch.index.compositeindex.datacube.startree.utils;
1111

12+
import org.apache.lucene.index.DocValues;
1213
import org.apache.lucene.index.SortedNumericDocValues;
1314
import org.apache.lucene.search.DocIdSetIterator;
1415
import org.opensearch.common.annotation.ExperimentalApi;
@@ -42,6 +43,14 @@ public SequentialDocValuesIterator(DocIdSetIterator docIdSetIterator) {
4243
this.docIdSetIterator = docIdSetIterator;
4344
}
4445

46+
/**
47+
* Constructs a new SequentialDocValuesIterator instance with an empty sorted numeric.
48+
*
49+
*/
50+
public SequentialDocValuesIterator() {
51+
this.docIdSetIterator = DocValues.emptySortedNumeric();
52+
}
53+
4554
/**
4655
* Returns the id of the latest document.
4756
*

‎server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public void setup() {
3636
public static Collection<Object[]> parameters() {
3737
List<Object[]> parameters = new ArrayList<>();
3838
for (StarTreeNumericType starTreeNumericType : StarTreeNumericType.values()) {
39-
parameters.add(new Object[] { starTreeNumericType });
39+
parameters.add(new Object[]{starTreeNumericType});
4040
}
4141
return parameters;
4242
}
@@ -48,7 +48,11 @@ public void testGetInitialAggregatedValueForSegmentDocNullValue() {
4848
}
4949

5050
public void testMergeAggregatedNullValueAndSegmentNullValue() {
51-
assertEquals(aggregator.getIdentityMetricValue(), aggregator.mergeAggregatedValueAndSegmentValue(null, null));
51+
if (aggregator instanceof CountValueAggregator) {
52+
assertThrows(AssertionError.class, () -> aggregator.mergeAggregatedValueAndSegmentValue(null, null));
53+
} else {
54+
assertEquals(aggregator.getIdentityMetricValue(), aggregator.mergeAggregatedValueAndSegmentValue(null, null));
55+
}
5256
}
5357

5458
public void testMergeAggregatedNullValues() {

‎server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ public void test_sortAndAggregateStarTreeDocuments_nullDimensionsAndNullMetrics(
453453
starTreeDocuments[4] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { null, null, null, null, null });
454454

455455
List<StarTreeDocument> inorderStarTreeDocuments = List.of(
456-
new StarTreeDocument(new Long[] { null, null, null, null }, new Object[] { 0.0, 0.0, 5L })
456+
new StarTreeDocument(new Long[] { null, null, null, null }, new Object[] { 0.0, 0.0, 0L, null, null })
457457
);
458458
Iterator<StarTreeDocument> expectedStarTreeDocumentIterator = inorderStarTreeDocuments.iterator();
459459

@@ -469,11 +469,78 @@ public void test_sortAndAggregateStarTreeDocuments_nullDimensionsAndNullMetrics(
469469
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[2])
470470
: null;
471471
Long metric4 = starTreeDocuments[i].metrics[3] != null
472-
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[2])
472+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[3])
473473
: null;
474474
Long metric5 = starTreeDocuments[i].metrics[4] != null
475+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[4])
476+
: null;
477+
segmentStarTreeDocuments[i] = new StarTreeDocument(
478+
starTreeDocuments[i].dimensions,
479+
new Object[] { metric1, metric2, metric3, metric4, metric5 }
480+
);
481+
}
482+
SequentialDocValuesIterator[] dimsIterators = getDimensionIterators(segmentStarTreeDocuments);
483+
List<SequentialDocValuesIterator> metricsIterators = getMetricIterators(segmentStarTreeDocuments);
484+
builder = getStarTreeBuilder(compositeField, writeState, mapperService);
485+
Iterator<StarTreeDocument> segmentStarTreeDocumentIterator = builder.sortAndAggregateSegmentDocuments(
486+
dimsIterators,
487+
metricsIterators
488+
);
489+
490+
while (segmentStarTreeDocumentIterator.hasNext() && expectedStarTreeDocumentIterator.hasNext()) {
491+
StarTreeDocument resultStarTreeDocument = segmentStarTreeDocumentIterator.next();
492+
StarTreeDocument expectedStarTreeDocument = expectedStarTreeDocumentIterator.next();
493+
assertEquals(expectedStarTreeDocument.dimensions[0], resultStarTreeDocument.dimensions[0]);
494+
assertEquals(expectedStarTreeDocument.dimensions[1], resultStarTreeDocument.dimensions[1]);
495+
assertEquals(expectedStarTreeDocument.dimensions[2], resultStarTreeDocument.dimensions[2]);
496+
assertEquals(expectedStarTreeDocument.dimensions[3], resultStarTreeDocument.dimensions[3]);
497+
assertEquals(expectedStarTreeDocument.metrics[0], resultStarTreeDocument.metrics[0]);
498+
assertEquals(expectedStarTreeDocument.metrics[1], resultStarTreeDocument.metrics[1]);
499+
assertEquals(expectedStarTreeDocument.metrics[2], resultStarTreeDocument.metrics[2]);
500+
assertEquals(expectedStarTreeDocument.metrics[3], resultStarTreeDocument.metrics[3]);
501+
assertEquals(expectedStarTreeDocument.metrics[4], resultStarTreeDocument.metrics[4]);
502+
}
503+
builder.build(segmentStarTreeDocumentIterator);
504+
validateStarTree(builder.getRootNode(), 4, 1, builder.getStarTreeDocuments());
505+
}
506+
507+
public void test_sortAndAggregateStarTreeDocuments_nullDimensionsAndFewNullMetrics() throws IOException {
508+
int noOfStarTreeDocuments = 5;
509+
StarTreeDocument[] starTreeDocuments = new StarTreeDocument[noOfStarTreeDocuments];
510+
511+
double sumValue = randomDouble();
512+
double minValue = randomDouble();
513+
double maxValue = randomDouble();
514+
515+
// Setting second metric iterator as empty sorted numeric , indicating a metric field is null
516+
starTreeDocuments[0] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { null, null, randomDouble(), null, maxValue });
517+
starTreeDocuments[1] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { null, null, null, null, null });
518+
starTreeDocuments[2] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { null, null, null, minValue, null });
519+
starTreeDocuments[3] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { null, null, null, null, null });
520+
starTreeDocuments[4] = new StarTreeDocument(new Long[] { null, null, null, null }, new Double[] { sumValue, null, randomDouble(), null, null });
521+
522+
List<StarTreeDocument> inorderStarTreeDocuments = List.of(
523+
new StarTreeDocument(new Long[] { null, null, null, null }, new Object[] { sumValue, 0.0, 2L, minValue, maxValue })
524+
);
525+
Iterator<StarTreeDocument> expectedStarTreeDocumentIterator = inorderStarTreeDocuments.iterator();
526+
527+
StarTreeDocument[] segmentStarTreeDocuments = new StarTreeDocument[noOfStarTreeDocuments];
528+
for (int i = 0; i < noOfStarTreeDocuments; i++) {
529+
Long metric1 = starTreeDocuments[i].metrics[0] != null
530+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[0])
531+
: null;
532+
Long metric2 = starTreeDocuments[i].metrics[1] != null
533+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[1])
534+
: null;
535+
Long metric3 = starTreeDocuments[i].metrics[2] != null
475536
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[2])
476537
: null;
538+
Long metric4 = starTreeDocuments[i].metrics[3] != null
539+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[3])
540+
: null;
541+
Long metric5 = starTreeDocuments[i].metrics[4] != null
542+
? NumericUtils.doubleToSortableLong((Double) starTreeDocuments[i].metrics[4])
543+
: null;
477544
segmentStarTreeDocuments[i] = new StarTreeDocument(
478545
starTreeDocuments[i].dimensions,
479546
new Object[] { metric1, metric2, metric3, metric4, metric5 }

0 commit comments

Comments
 (0)