addressing comments

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

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

@@ -27,9 +27,9 @@
 import org.opensearch.index.compositeindex.datacube.MetricStat;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
+import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
 import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitAdapter;
 import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
-import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
 import org.opensearch.index.query.QueryBuilders;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.search.SearchHit;
@@ -392,7 +392,7 @@ public void testValidCompositeIndex() {
                     DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
                     List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
                         new DateTimeUnitAdapter(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
-                        ExtendedDateTimeUnit.HALF_HOUR_OF_DAY
+                        DataCubeDateTimeUnit.HALF_HOUR_OF_DAY
                     );
                     for (int i = 0; i < dateDim.getSortedCalendarIntervals().size(); i++) {
                         assertEquals(expectedTimeUnits.get(i).shortName(), dateDim.getSortedCalendarIntervals().get(i).shortName());
@@ -428,8 +428,8 @@ public void testValidCompositeIndexWithDates() {
                     assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension);
                     DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
                     List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
-                        ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY,
-                        ExtendedDateTimeUnit.HALF_HOUR_OF_DAY,
+                        DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY,
+                        DataCubeDateTimeUnit.HALF_HOUR_OF_DAY,
                         new DateTimeUnitAdapter(Rounding.DateTimeUnit.DAY_OF_MONTH)
                     );
                     for (int i = 0; i < dateDim.getIntervals().size(); i++) {
@@ -466,7 +466,7 @@ public void testValidCompositeIndexWithDuplicateDates() {
                     assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension);
                     DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
                     List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
-                        ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY,
+                        DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY,
                         new DateTimeUnitAdapter(Rounding.DateTimeUnit.DAY_OF_MONTH)
                     );
                     for (int i = 0; i < dateDim.getIntervals().size(); i++) {
@@ -637,7 +637,7 @@ public void testUpdateIndexWhenMappingIsSame() {
                     DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0);
                     List<DateTimeUnitRounding> expectedTimeUnits = Arrays.asList(
                         new DateTimeUnitAdapter(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
-                        ExtendedDateTimeUnit.HALF_HOUR_OF_DAY
+                        DataCubeDateTimeUnit.HALF_HOUR_OF_DAY
                     );
                     for (int i = 0; i < expectedTimeUnits.size(); i++) {
                         assertEquals(expectedTimeUnits.get(i).shortName(), dateDim.getIntervals().get(i).shortName());

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

@@ -12,8 +12,8 @@
 import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.common.time.DateUtils;
 import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
 import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
-import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
 import org.opensearch.index.mapper.CompositeDataCubeFieldType;
 import org.opensearch.index.mapper.DateFieldMapper;
 
@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 /**
@@ -64,26 +65,24 @@ public List<DateTimeUnitRounding> getSortedCalendarIntervals() {
      * Sets the dimension values in sorted order in the provided array starting from the given index.
      *
      * @param val   The value to be set
-     * @param dims  The dimensions array to set the values in
-     * @param index The starting index in the array
-     * @return The next available index in the array
+     * @param dimSetter  Consumer which sets the dimensions
      */
     @Override
-    public int setDimensionValues(final Long val, final Long[] dims, int index) {
+    public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
         for (DateTimeUnitRounding dateTimeUnit : sortedCalendarIntervals) {
             if (val == null) {
-                dims[index++] = null;
-                continue;
+                dimSetter.accept(null);
+            } else {
+                Long roundedValue = dateTimeUnit.roundFloor(storedDurationSinceEpoch(val));
+                dimSetter.accept(roundedValue);
             }
-            dims[index++] = dateTimeUnit.roundFloor(maybeConvertNanosToMillis(val));
         }
-        return index;
     }
 
     /**
      * Converts nanoseconds to milliseconds based on the resolution of the field
      */
-    private long maybeConvertNanosToMillis(long nanoSecondsSinceEpoch) {
+    private long storedDurationSinceEpoch(long nanoSecondsSinceEpoch) {
         if (resolution.equals(DateFieldMapper.Resolution.NANOSECONDS)) return DateUtils.toMilliSeconds(nanoSecondsSinceEpoch);
         return nanoSecondsSinceEpoch;
     }
@@ -92,7 +91,7 @@ private long maybeConvertNanosToMillis(long nanoSecondsSinceEpoch) {
      * Returns the list of fields that represent the dimension
      */
     @Override
-    public List<String> getDimensionFieldsNames() {
+    public List<String> getSubDimensionNames() {
         List<String> fields = new ArrayList<>(calendarIntervals.size());
         for (DateTimeUnitRounding interval : sortedCalendarIntervals) {
             // TODO : revisit this post file format changes
@@ -147,8 +146,8 @@ public static class DateTimeUnitComparator implements Comparator<DateTimeUnitRou
         static {
             ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.SECOND_OF_MINUTE.shortName(), 1);
             ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), 2);
-            ORDERED_DATE_TIME_UNIT.put(ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY.shortName(), 3);
-            ORDERED_DATE_TIME_UNIT.put(ExtendedDateTimeUnit.HALF_HOUR_OF_DAY.shortName(), 4);
+            ORDERED_DATE_TIME_UNIT.put(DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY.shortName(), 3);
+            ORDERED_DATE_TIME_UNIT.put(DataCubeDateTimeUnit.HALF_HOUR_OF_DAY.shortName(), 4);
             ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.HOUR_OF_DAY.shortName(), 5);
             ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.DAY_OF_MONTH.shortName(), 6);
             ORDERED_DATE_TIME_UNIT.put(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR.shortName(), 7);

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

@@ -12,6 +12,7 @@
 import org.opensearch.core.xcontent.ToXContent;
 
 import java.util.List;
+import java.util.function.Consumer;
 
 /**
  * Base interface for data-cube dimensions
@@ -30,17 +31,15 @@ public interface Dimension extends ToXContent {
     int getNumSubDimensions();
 
     /**
-     * Sets the dimension values in the provided array starting from the given index.
+     * Sets the dimension values with the consumer
      *
      * @param value   The value to be set
-     * @param dims  The dimensions array to set the values in
-     * @param index The starting index in the array
-     * @return The next available index in the array
+     * @param dimSetter  Consumer which sets the dimensions
      */
-    int setDimensionValues(Long value, Long[] dims, int index);
+    void setDimensionValues(final Long value, final Consumer<Long> dimSetter);
 
     /**
      * Returns the list of dimension fields that represent the dimension
      */
-    List<String> getDimensionFieldsNames();
+    List<String> getSubDimensionNames();
 }

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

@@ -15,6 +15,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.Consumer;
 
 /**
  * Composite index numeric dimension class
@@ -40,13 +41,12 @@ public int getNumSubDimensions() {
     }
 
     @Override
-    public int setDimensionValues(final Long val, final Long[] dims, int index) {
-        dims[index++] = val;
-        return index;
+    public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
+        dimSetter.accept(val);
     }
 
     @Override
-    public List<String> getDimensionFieldsNames() {
+    public List<String> getSubDimensionNames() {
         // TODO : revisit this post file format changes
         return List.of(field);
     }

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

@@ -14,6 +14,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.Consumer;
 
 /**
  * Represents a dimension for reconstructing StarTreeField from file formats during searches and merges.
@@ -38,13 +39,12 @@ public int getNumSubDimensions() {
     }
 
     @Override
-    public int setDimensionValues(Long value, Long[] dims, int index) {
-        dims[index++] = value;
-        return index;
+    public void setDimensionValues(final Long val, final Consumer<Long> dimSetter) {
+        dimSetter.accept(val);
     }
 
     @Override
-    public List<String> getDimensionFieldsNames() {
+    public List<String> getSubDimensionNames() {
         return List.of(field);
     }
 

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

@@ -42,7 +42,7 @@ public StarTreeField(String name, List<Dimension> dimensions, List<Metric> metri
         this.starTreeConfig = starTreeConfig;
         dimensionNames = new ArrayList<>();
         for (Dimension dimension : dimensions) {
-            dimensionNames.addAll(dimension.getDimensionFieldsNames());
+            dimensionNames.addAll(dimension.getSubDimensionNames());
         }
         metricNames = new ArrayList<>();
         for (Metric metric : metrics) {

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

@@ -11,9 +11,9 @@
 import org.opensearch.common.Rounding;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.index.compositeindex.datacube.MetricStat;
+import org.opensearch.index.compositeindex.datacube.startree.utils.date.DataCubeDateTimeUnit;
 import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitAdapter;
 import org.opensearch.index.compositeindex.datacube.startree.utils.date.DateTimeUnitRounding;
-import org.opensearch.index.compositeindex.datacube.startree.utils.date.ExtendedDateTimeUnit;
 import org.opensearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder;
 
 import java.util.Arrays;
@@ -102,7 +102,7 @@ public class StarTreeIndexSettings {
      */
     public static final Setting<List<DateTimeUnitRounding>> DEFAULT_DATE_INTERVALS = Setting.listSetting(
         "index.composite_index.star_tree.field.default.date_intervals",
-        Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), ExtendedDateTimeUnit.HALF_HOUR_OF_DAY.shortName()),
+        Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), DataCubeDateTimeUnit.HALF_HOUR_OF_DAY.shortName()),
         StarTreeIndexSettings::getTimeUnit,
         Setting.Property.IndexScope,
         Setting.Property.Final
@@ -122,8 +122,8 @@ public class StarTreeIndexSettings {
     public static DateTimeUnitRounding getTimeUnit(String expression) {
         if (DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(expression)) {
             return new DateTimeUnitAdapter(DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression));
-        } else if (ExtendedDateTimeUnit.DATE_FIELD_UNITS.containsKey(expression)) {
-            return ExtendedDateTimeUnit.DATE_FIELD_UNITS.get(expression);
+        } else if (DataCubeDateTimeUnit.DATE_FIELD_UNITS.containsKey(expression)) {
+            return DataCubeDateTimeUnit.DATE_FIELD_UNITS.get(expression);
         }
         throw new IllegalArgumentException("unknown calendar intervals specified in star tree index mapping");
     }

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

@@ -325,7 +325,7 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer, A
         FieldInfo[] metricFieldInfoList = new FieldInfo[metricAggregatorInfos.size()];
         int dimIndex = 0;
         for (Dimension dim : dimensionsSplitOrder) {
-            for (String name : dim.getDimensionFieldsNames()) {
+            for (String name : dim.getSubDimensionNames()) {
                 final FieldInfo fi = getFieldInfo(
                     fullyQualifiedFieldNameForStarTreeDimensionsDocValues(starTreeField.getName(), name),
                     DocValuesType.SORTED_NUMERIC,
@@ -539,7 +539,7 @@ protected StarTreeDocument getSegmentStarTreeDocument(
      */
     Long[] getStarTreeDimensionsFromSegment(int currentDocId, SequentialDocValuesIterator[] dimensionReaders) throws IOException {
         Long[] dimensions = new Long[numDimensions];
-        int dimIndex = 0;
+        AtomicInteger dimIndex = new AtomicInteger(0);
         for (int i = 0; i < dimensionReaders.length; i++) {
             if (dimensionReaders[i] != null) {
                 try {
@@ -551,11 +551,16 @@ Long[] getStarTreeDimensionsFromSegment(int currentDocId, SequentialDocValuesIte
                     logger.error("unable to read the dimension values from the segment", e);
                     throw new IllegalStateException("unable to read the dimension values from the segment", e);
                 }
-                dimIndex = dimensionsSplitOrder.get(i).setDimensionValues(dimensionReaders[i].value(currentDocId), dimensions, dimIndex);
+                dimensionsSplitOrder.get(i).setDimensionValues(dimensionReaders[i].value(currentDocId), value -> {
+                    dimensions[dimIndex.getAndIncrement()] = value;
+                });
             } else {
                 throw new IllegalStateException("dimension readers are empty");
             }
         }
+        if (dimIndex.get() == numDimensions) {
+            throw new IllegalStateException("Values are not set for all dimensions");
+        }
         return dimensions;
     }
 

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

@@ -28,7 +28,7 @@
  *
  * @opensearch.experimental
  */
-public enum ExtendedDateTimeUnit implements DateTimeUnitRounding {
+public enum DataCubeDateTimeUnit implements DateTimeUnitRounding {
     HALF_HOUR_OF_DAY("half-hour") {
         @Override
         public long roundFloor(long utcMillis) {
@@ -42,19 +42,19 @@ public long roundFloor(long utcMillis) {
         }
     };
 
-    public static final Map<String, ExtendedDateTimeUnit> DATE_FIELD_UNITS;
+    public static final Map<String, DataCubeDateTimeUnit> DATE_FIELD_UNITS;
     static {
-        Map<String, ExtendedDateTimeUnit> dateFieldUnits = new HashMap<>();
-        dateFieldUnits.put("30m", ExtendedDateTimeUnit.HALF_HOUR_OF_DAY);
-        dateFieldUnits.put("half-hour", ExtendedDateTimeUnit.HALF_HOUR_OF_DAY);
-        dateFieldUnits.put("15m", ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY);
-        dateFieldUnits.put("quarter-hour", ExtendedDateTimeUnit.QUARTER_HOUR_OF_DAY);
+        Map<String, DataCubeDateTimeUnit> dateFieldUnits = new HashMap<>();
+        dateFieldUnits.put("30m", DataCubeDateTimeUnit.HALF_HOUR_OF_DAY);
+        dateFieldUnits.put("half-hour", DataCubeDateTimeUnit.HALF_HOUR_OF_DAY);
+        dateFieldUnits.put("15m", DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY);
+        dateFieldUnits.put("quarter-hour", DataCubeDateTimeUnit.QUARTER_HOUR_OF_DAY);
         DATE_FIELD_UNITS = unmodifiableMap(dateFieldUnits);
     }
 
     private final String shortName;
 
-    ExtendedDateTimeUnit(String shortName) {
+    DataCubeDateTimeUnit(String shortName) {
         this.shortName = shortName;
     }