Skip to content

Commit 05ee1f5

Browse files
sarthakaggarwal97sarthakaggarwal97
committed
addressing comments
Signed-off-by: Sarthak Aggarwal <[email protected]>
1 parent ea7a0d4 commit 05ee1f5

File tree

6 files changed

+13
-73
lines changed

6 files changed

+13
-73
lines changed

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,7 @@
1212

1313
public class StarTreeTestUtils {
1414

15-
public static Double toStarTreeNumericTypeValue(Long value, StarTreeNumericType starTreeNumericType) {
16-
try {
17-
return starTreeNumericType.getDoubleValue(value);
18-
} catch (Exception e) {
19-
throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e);
20-
}
15+
public static Double toDoubleValue(Long value, StarTreeNumericType starTreeNumericType) {
16+
return starTreeNumericType.getDoubleValue(value);
2117
}
2218
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void testGetInitialAggregatedValueForSegmentDocValue() {
7070
assertEquals(CountValueAggregator.DEFAULT_INITIAL_VALUE, aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong()));
7171
} else {
7272
assertEquals(
73-
StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
73+
StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
7474
aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong)
7575
);
7676
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,18 @@ public void testMergeAggregatedValueAndSegmentValue() {
2424
Long randomLong = randomLong();
2525
double randomDouble = randomDouble();
2626
assertEquals(
27-
Math.max(StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType), randomDouble),
27+
Math.max(StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType), randomDouble),
2828
aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
2929
0.0
3030
);
3131
assertEquals(
32-
StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
32+
StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
3333
aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
3434
0.0
3535
);
3636
assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0);
3737
assertEquals(
38-
Math.max(2.0, StarTreeTestUtils.toStarTreeNumericTypeValue(3L, starTreeNumericType)),
38+
Math.max(2.0, StarTreeTestUtils.toDoubleValue(3L, starTreeNumericType)),
3939
aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L),
4040
0.0
4141
);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ public void testMergeAggregatedValueAndSegmentValue() {
2323
Long randomLong = randomLong();
2424
double randomDouble = randomDouble();
2525
assertEquals(
26-
Math.min(StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType), randomDouble),
26+
Math.min(StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType), randomDouble),
2727
aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
2828
0.0
2929
);
3030
assertEquals(
31-
StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
31+
StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
3232
aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
3333
0.0
3434
);
3535
assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0);
3636
assertEquals(
37-
Math.min(2.0, StarTreeTestUtils.toStarTreeNumericTypeValue(3L, starTreeNumericType)),
37+
Math.min(2.0, StarTreeTestUtils.toDoubleValue(3L, starTreeNumericType)),
3838
aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L),
3939
0.0
4040
);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public void testMergeAggregatedValueAndSegmentValue() {
3030
Long randomLong = randomLong();
3131
aggregator.getInitialAggregatedValue(randomDouble);
3232
assertEquals(
33-
randomDouble + StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
33+
randomDouble + StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
3434
aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
3535
0.0
3636
);
@@ -42,7 +42,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullSegmentDocValue() {
4242
aggregator.getInitialAggregatedValue(randomDouble1);
4343
assertEquals(randomDouble1, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, null), 0.0);
4444
assertEquals(
45-
randomDouble1 + StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
45+
randomDouble1 + StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
4646
aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, randomLong),
4747
0.0
4848
);
@@ -52,7 +52,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullInitialDocValue() {
5252
Long randomLong = randomLong();
5353
aggregator.getInitialAggregatedValue(null);
5454
assertEquals(
55-
StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
55+
StarTreeTestUtils.toDoubleValue(randomLong, starTreeNumericType),
5656
aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
5757
0.0
5858
);

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

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,22 +1509,6 @@ private static StarTreeField getStarTreeFieldWithMultipleMetrics() {
15091509
}
15101510

15111511
public void testMergeFlowWithSum_randomNumberTypes() throws IOException {
1512-
List<Long> dimList = List.of(0L, 1L, 3L, 4L, 5L, 6L);
1513-
List<Integer> docsWithField = List.of(0, 1, 3, 4, 5, 6);
1514-
List<Long> dimList2 = List.of(0L, 1L, 2L, 3L, 4L, 5L, -1L);
1515-
List<Integer> docsWithField2 = List.of(0, 1, 2, 3, 4, 5, 6);
1516-
1517-
List<Long> metricsList = List.of(
1518-
getLongFromDouble(0.0),
1519-
getLongFromDouble(10.0),
1520-
getLongFromDouble(20.0),
1521-
getLongFromDouble(30.0),
1522-
getLongFromDouble(40.0),
1523-
getLongFromDouble(50.0),
1524-
getLongFromDouble(60.0)
1525-
1526-
);
1527-
List<Integer> metricsWithField = List.of(0, 1, 2, 3, 4, 5, 6);
15281512

15291513
DocumentMapper documentMapper = mock(DocumentMapper.class);
15301514
when(mapperService.documentMapper()).thenReturn(documentMapper);
@@ -1555,48 +1539,8 @@ public void testMergeFlowWithSum_randomNumberTypes() throws IOException {
15551539
null
15561540
);
15571541
when(documentMapper.mappers()).thenReturn(fieldMappers);
1542+
testMergeFlowWithSum();
15581543

1559-
StarTreeField sf = getStarTreeField(MetricStat.SUM);
1560-
StarTreeValues starTreeValues = getStarTreeValues(
1561-
getSortedNumericMock(dimList, docsWithField),
1562-
getSortedNumericMock(dimList2, docsWithField2),
1563-
getSortedNumericMock(metricsList, metricsWithField),
1564-
sf,
1565-
"6"
1566-
);
1567-
1568-
StarTreeValues starTreeValues2 = getStarTreeValues(
1569-
getSortedNumericMock(dimList, docsWithField),
1570-
getSortedNumericMock(dimList2, docsWithField2),
1571-
getSortedNumericMock(metricsList, metricsWithField),
1572-
sf,
1573-
"6"
1574-
);
1575-
builder = getStarTreeBuilder(sf, getWriteState(6), mapperService);
1576-
Iterator<StarTreeDocument> starTreeDocumentIterator = builder.mergeStarTrees(List.of(starTreeValues, starTreeValues2));
1577-
/**
1578-
* Asserting following dim / metrics [ dim1, dim2 / Sum [ metric] ]
1579-
* [0, 0] | [0.0]
1580-
* [1, 1] | [20.0]
1581-
* [3, 3] | [60.0]
1582-
* [4, 4] | [80.0]
1583-
* [5, 5] | [100.0]
1584-
* [null, 2] | [40.0]
1585-
* ------------------ We only take non star docs
1586-
* [6,-1] | [120.0]
1587-
*/
1588-
int count = 0;
1589-
while (starTreeDocumentIterator.hasNext()) {
1590-
count++;
1591-
StarTreeDocument starTreeDocument = starTreeDocumentIterator.next();
1592-
assertEquals(
1593-
starTreeDocument.dimensions[0] != null ? starTreeDocument.dimensions[0] * 2 * 10.0 : 40.0,
1594-
starTreeDocument.metrics[0]
1595-
);
1596-
}
1597-
assertEquals(6, count);
1598-
builder.build(starTreeDocumentIterator);
1599-
validateStarTree(builder.getRootNode(), 2, 1, builder.getStarTreeDocuments());
16001544
}
16011545

16021546
public void testMergeFlowWithSum() throws IOException {

0 commit comments

Comments
 (0)