Skip to content

Commit 93760bc

Browse files
bharath-techiebharath-techie
committed
addressing comments
Signed-off-by: Bharathwaj G <[email protected]>
1 parent ffb6426 commit 93760bc

File tree

13 files changed

+119
-94
lines changed

13 files changed

+119
-94
lines changed

‎server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
import org.opensearch.index.compositeindex.datacube.MetricStat;
2828
import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
2929
import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
30+
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
3031
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitAdapter;
3132
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
32-
import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
3333
import org.opensearch.index.query.QueryBuilders;
3434
import org.opensearch.indices.IndicesService;
3535
import org.opensearch.search.SearchHit;
@@ -392,7 +392,7 @@ public void testValidCompositeIndex() {
392392
DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
393393
List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
394394
new DateTimeUnitAdapter(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
395-
ExtendedDateTimeUnit.HALF_HOUR_OF_DAY
395+
DataCubeDateTimeUnit.HALF_HOUR_OF_DAY
396396
);
397397
for (int i = 0; i < dateDim.getSortedCalendarIntervals().size(); i++) {
398398
assertEquals(expectedTimeUnits.get(i).shortName(), dateDim.getSortedCalendarIntervals().get(i).shortName());
@@ -428,8 +428,8 @@ public void testValidCompositeIndexWithDates() {
428428
assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension);
429429
DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
430430
List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
431-
ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY,
432-
ExtendedDateTimeUnit.HALF_HOUR_OF_DAY,
431+
DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY,
432+
DataCubeDateTimeUnit.HALF_HOUR_OF_DAY,
433433
new DateTimeUnitAdapter(Rounding.DateTimeUnit.DAY_OF_MONTH)
434434
);
435435
for (int i = 0; i < dateDim.getIntervals().size(); i++) {
@@ -466,7 +466,7 @@ public void testValidCompositeIndexWithDuplicateDates() {
466466
assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension);
467467
DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
468468
List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
469-
ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY,
469+
DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY,
470470
new DateTimeUnitAdapter(Rounding.DateTimeUnit.DAY_OF_MONTH)
471471
);
472472
for (int i = 0; i < dateDim.getIntervals().size(); i++) {
@@ -637,7 +637,7 @@ public void testUpdateIndexWhenMappingIsSame() {
637637
DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
638638
List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
639639
new DateTimeUnitAdapter(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
640-
ExtendedDateTimeUnit.HALF_HOUR_OF_DAY
640+
DataCubeDateTimeUnit.HALF_HOUR_OF_DAY
641641
);
642642
for (int i = 0; i < expectedTimeUnits.size(); i++) {
643643
assertEquals(expectedTimeUnits.get(i).shortName(), dateDim.getIntervals().get(i).shortName());

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/DateDimension.java

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
import org.opensearch.common.annotation.ExperimentalApi;
1313
import org.opensearch.common.time.DateUtils;
1414
import org.opensearch.core.xcontent.XContentBuilder;
15+
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
1516
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
16-
import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
1717
import org.opensearch.index.mapper.CompositeDataCubeFieldType;
1818
import org.opensearch.index.mapper.DateFieldMapper;
1919

@@ -24,6 +24,7 @@
2424
import java.util.List;
2525
import java.util.Map;
2626
import java.util.Objects;
27+
import java.util.function.Consumer;
2728
import java.util.stream.Collectors;
2829

2930
/**
@@ -64,26 +65,24 @@ public List<DateTimeUnitRounding> getSortedCalendarIntervals() {
6465
* Sets the dimension values in sorted order in the provided array starting from the given index.
6566
*
6667
* @param val The value to be set
67-
* @param dims The dimensions array to set the values in
68-
* @param index The starting index in the array
69-
* @return The next available index in the array
68+
* @param dimSetter Consumer which sets the dimensions
7069
*/
7170
@Override
72-
public int setDimensionValues(final Long val, final Long[] dims, int index) {
71+
public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
7372
for (DateTimeUnitRounding dateTimeUnit : sortedCalendarIntervals) {
7473
if (val == null) {
75-
dims[index++] = null;
76-
continue;
74+
dimSetter.accept(null);
75+
} else {
76+
Long roundedValue = dateTimeUnit.roundFloor(storedDurationSinceEpoch(val));
77+
dimSetter.accept(roundedValue);
7778
}
78-
dims[index++] = dateTimeUnit.roundFloor(maybeConvertNanosToMillis(val));
7979
}
80-
return index;
8180
}
8281

8382
/**
8483
* Converts nanoseconds to milliseconds based on the resolution of the field
8584
*/
86-
private long maybeConvertNanosToMillis(long nanoSecondsSinceEpoch) {
85+
private long storedDurationSinceEpoch(long nanoSecondsSinceEpoch) {
8786
if (resolution.equals(DateFieldMapper.Resolution.NANOSECONDS)) return DateUtils.toMilliSeconds(nanoSecondsSinceEpoch);
8887
return nanoSecondsSinceEpoch;
8988
}
@@ -92,7 +91,7 @@ private long maybeConvertNanosToMillis(long nanoSecondsSinceEpoch) {
9291
* Returns the list of fields that represent the dimension
9392
*/
9493
@Override
95-
public List<String> getDimensionFieldsNames() {
94+
public List<String> getSubDimensionNames() {
9695
List<String> fields = new ArrayList<>(calendarIntervals.size());
9796
for (DateTimeUnitRounding interval : sortedCalendarIntervals) {
9897
// TODO : revisit this post file format changes
@@ -147,8 +146,8 @@ public static class DateTimeUnitComparator implements Comparator<DateTimeUnitRou
147146
static {
148147
ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.SECOND_OF_MINUTE.shortName(), 1);
149148
ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), 2);
150-
ORDERED_DATE_TIME_UNIT.put(ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY.shortName(), 3);
151-
ORDERED_DATE_TIME_UNIT.put(ExtendedDateTimeUnit.HALF_HOUR_OF_DAY.shortName(), 4);
149+
ORDERED_DATE_TIME_UNIT.put(DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY.shortName(), 3);
150+
ORDERED_DATE_TIME_UNIT.put(DataCubeDateTimeUnit.HALF_HOUR_OF_DAY.shortName(), 4);
152151
ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.HOUR_OF_DAY.shortName(), 5);
153152
ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.DAY_OF_MONTH.shortName(), 6);
154153
ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR.shortName(), 7);

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/Dimension.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.opensearch.core.xcontent.ToXContent;
1313

1414
import java.util.List;
15+
import java.util.function.Consumer;
1516

1617
/**
1718
* Base interface for data-cube dimensions
@@ -30,17 +31,15 @@ public interface Dimension extends ToXContent {
3031
int getNumSubDimensions();
3132

3233
/**
33-
* Sets the dimension values in the provided array starting from the given index.
34+
* Sets the dimension values with the consumer
3435
*
3536
* @param value The value to be set
36-
* @param dims The dimensions array to set the values in
37-
* @param index The starting index in the array
38-
* @return The next available index in the array
37+
* @param dimSetter Consumer which sets the dimensions
3938
*/
40-
int setDimensionValues(Long value, Long[] dims, int index);
39+
void setDimensionValues(final Long value, final Consumer<Long> dimSetter);
4140

4241
/**
4342
* Returns the list of dimension fields that represent the dimension
4443
*/
45-
List<String> getDimensionFieldsNames();
44+
List<String> getSubDimensionNames();
4645
}

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/NumericDimension.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.IOException;
1616
import java.util.List;
1717
import java.util.Objects;
18+
import java.util.function.Consumer;
1819

1920
/**
2021
* Composite index numeric dimension class
@@ -40,13 +41,12 @@ public int getNumSubDimensions() {
4041
}
4142

4243
@Override
43-
public int setDimensionValues(final Long val, final Long[] dims, int index) {
44-
dims[index++] = val;
45-
return index;
44+
public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
45+
dimSetter.accept(val);
4646
}
4747

4848
@Override
49-
public List<String> getDimensionFieldsNames() {
49+
public List<String> getSubDimensionNames() {
5050
// TODO : revisit this post file format changes
5151
return List.of(field);
5252
}

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/ReadDimension.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.io.IOException;
1515
import java.util.List;
1616
import java.util.Objects;
17+
import java.util.function.Consumer;
1718

1819
/**
1920
* Represents a dimension for reconstructing StarTreeField from file formats during searches and merges.
@@ -38,13 +39,12 @@ public int getNumSubDimensions() {
3839
}
3940

4041
@Override
41-
public int setDimensionValues(Long value, Long[] dims, int index) {
42-
dims[index++] = value;
43-
return index;
42+
public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
43+
dimSetter.accept(val);
4444
}
4545

4646
@Override
47-
public List<String> getDimensionFieldsNames() {
47+
public List<String> getSubDimensionNames() {
4848
return List.of(field);
4949
}
5050

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public StarTreeField(String name, List<Dimension> dimensions, List<Metric> metri
4242
this.starTreeConfig = starTreeConfig;
4343
dimensionNames = new ArrayList<>();
4444
for (Dimension dimension : dimensions) {
45-
dimensionNames.addAll(dimension.getDimensionFieldsNames());
45+
dimensionNames.addAll(dimension.getSubDimensionNames());
4646
}
4747
metricNames = new ArrayList<>();
4848
for (Metric metric : metrics) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111
import org.opensearch.common.Rounding;
1212
import org.opensearch.common.settings.Setting;
1313
import org.opensearch.index.compositeindex.datacube.MetricStat;
14+
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
1415
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitAdapter;
1516
import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
16-
import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
1717
import org.opensearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder;
1818

1919
import java.util.Arrays;
@@ -102,7 +102,7 @@ public class StarTreeIndexSettings {
102102
*/
103103
public static final Setting<List<DateTimeUnitRounding>> DEFAULT_DATE_INTERVALS = Setting.listSetting(
104104
"index.composite_index.star_tree.field.default.date_intervals",
105-
Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), ExtendedDateTimeUnit.HALF_HOUR_OF_DAY.shortName()),
105+
Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), DataCubeDateTimeUnit.HALF_HOUR_OF_DAY.shortName()),
106106
StarTreeIndexSettings::getTimeUnit,
107107
Setting.Property.IndexScope,
108108
Setting.Property.Final
@@ -122,8 +122,8 @@ public class StarTreeIndexSettings {
122122
public static DateTimeUnitRounding getTimeUnit(String expression) {
123123
if (DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(expression)) {
124124
return new DateTimeUnitAdapter(DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression));
125-
} else if (ExtendedDateTimeUnit.DATE_FIELD_UNITS.containsKey(expression)) {
126-
return ExtendedDateTimeUnit.DATE_FIELD_UNITS.get(expression);
125+
} else if (DataCubeDateTimeUnit.DATE_FIELD_UNITS.containsKey(expression)) {
126+
return DataCubeDateTimeUnit.DATE_FIELD_UNITS.get(expression);
127127
}
128128
throw new IllegalArgumentException("unknown calendar intervals specified in star tree index mapping");
129129
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer, A
325325
FieldInfo[] metricFieldInfoList = new FieldInfo[metricAggregatorInfos.size()];
326326
int dimIndex = 0;
327327
for (Dimension dim : dimensionsSplitOrder) {
328-
for (String name : dim.getDimensionFieldsNames()) {
328+
for (String name : dim.getSubDimensionNames()) {
329329
final FieldInfo fi = getFieldInfo(
330330
fullyQualifiedFieldNameForStarTreeDimensionsDocValues(starTreeField.getName(), name),
331331
DocValuesType.SORTED_NUMERIC,
@@ -539,7 +539,7 @@ protected StarTreeDocument getSegmentStarTreeDocument(
539539
*/
540540
Long[] getStarTreeDimensionsFromSegment(int currentDocId, SequentialDocValuesIterator[] dimensionReaders) throws IOException {
541541
Long[] dimensions = new Long[numDimensions];
542-
int dimIndex = 0;
542+
AtomicInteger dimIndex = new AtomicInteger(0);
543543
for (int i = 0; i < dimensionReaders.length; i++) {
544544
if (dimensionReaders[i] != null) {
545545
try {
@@ -551,11 +551,16 @@ Long[] getStarTreeDimensionsFromSegment(int currentDocId, SequentialDocValuesIte
551551
logger.error("unable to read the dimension values from the segment", e);
552552
throw new IllegalStateException("unable to read the dimension values from the segment", e);
553553
}
554-
dimIndex = dimensionsSplitOrder.get(i).setDimensionValues(dimensionReaders[i].value(currentDocId), dimensions, dimIndex);
554+
dimensionsSplitOrder.get(i).setDimensionValues(dimensionReaders[i].value(currentDocId), value -> {
555+
dimensions[dimIndex.getAndIncrement()] = value;
556+
});
555557
} else {
556558
throw new IllegalStateException("dimension readers are empty");
557559
}
558560
}
561+
if (dimIndex.get() == numDimensions) {
562+
throw new IllegalStateException("Values are not set for all dimensions");
563+
}
559564
return dimensions;
560565
}
561566

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/date/ExtendedDateTimeUnit.java renamed to ‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/date/DataCubeDateTimeUnit.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
*
2929
* @opensearch.experimental
3030
*/
31-
public enum ExtendedDateTimeUnit implements DateTimeUnitRounding {
31+
public enum DataCubeDateTimeUnit implements DateTimeUnitRounding {
3232
HALF_HOUR_OF_DAY("half-hour") {
3333
@Override
3434
public long roundFloor(long utcMillis) {
@@ -42,19 +42,19 @@ public long roundFloor(long utcMillis) {
4242
}
4343
};
4444

45-
public static final Map<String, ExtendedDateTimeUnit> DATE_FIELD_UNITS;
45+
public static final Map<String, DataCubeDateTimeUnit> DATE_FIELD_UNITS;
4646
static {
47-
Map<String, ExtendedDateTimeUnit> dateFieldUnits = new HashMap<>();
48-
dateFieldUnits.put("30m", ExtendedDateTimeUnit.HALF_HOUR_OF_DAY);
49-
dateFieldUnits.put("half-hour", ExtendedDateTimeUnit.HALF_HOUR_OF_DAY);
50-
dateFieldUnits.put("15m", ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY);
51-
dateFieldUnits.put("quarter-hour", ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY);
47+
Map<String, DataCubeDateTimeUnit> dateFieldUnits = new HashMap<>();
48+
dateFieldUnits.put("30m", DataCubeDateTimeUnit.HALF_HOUR_OF_DAY);
49+
dateFieldUnits.put("half-hour", DataCubeDateTimeUnit.HALF_HOUR_OF_DAY);
50+
dateFieldUnits.put("15m", DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY);
51+
dateFieldUnits.put("quarter-hour", DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY);
5252
DATE_FIELD_UNITS = unmodifiableMap(dateFieldUnits);
5353
}
5454

5555
private final String shortName;
5656

57-
ExtendedDateTimeUnit(String shortName) {
57+
DataCubeDateTimeUnit(String shortName) {
5858
this.shortName = shortName;
5959
}
6060

0 commit comments

Comments
 (0)