Refactoring

Signed-off-by: Sarthak Aggarwal <[email protected]>
opensearch-project · Jun 10, 2024 · c135dff · c135dff
1 parent feab228
commit c135dff
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 61 deletions.
diff --git a/server/src/main/java/org/apache/lucene/index/BaseCompositeFieldStarTreeBuilder.java b/server/src/main/java/org/apache/lucene/index/BaseCompositeFieldStarTreeBuilder.java
@@ -28,8 +28,9 @@
 import org.opensearch.index.compositeindex.startree.aggregators.ValueAggregator;
 import org.opensearch.index.compositeindex.startree.aggregators.ValueAggregatorFactory;
 import org.opensearch.index.compositeindex.startree.builder.CompositeFieldWriter;
-import org.opensearch.index.compositeindex.startree.builder.StarTreeDocValues;
+import org.opensearch.index.compositeindex.startree.data.StarTreeDocValues;
 import org.opensearch.index.compositeindex.startree.builder.StarTreeDocValuesIteratorFactory;
+import org.opensearch.index.compositeindex.startree.data.StarTreeDocument;
 import org.opensearch.index.compositeindex.startree.node.StarTreeNode;
 import org.opensearch.index.compositeindex.startree.utils.StarTreeBuilderUtils;
 import org.opensearch.index.mapper.NumberFieldMapper;
@@ -145,7 +146,7 @@ protected BaseCompositeFieldStarTreeBuilder(
         int index = 0;
         for (MetricTypeFieldPair metricTypeFieldPair : metricTypeFieldPairs) {
             metrics[index] = metricTypeFieldPair.toFieldName();
-            valueAggregators[index] = ValueAggregatorFactory.getValueAggregator(metricTypeFieldPair.getFunctionType());
+            valueAggregators[index] = ValueAggregatorFactory.getValueAggregator(metricTypeFieldPair.getMetricType());
             // Ignore the column for COUNT aggregation function
             if (valueAggregators[index].getAggregationType() != MetricType.COUNT) {
                 String metricName = metricTypeFieldPair.getField();
@@ -293,17 +294,17 @@ protected StarTreeDocument mergeSegmentStarTreeDocument(
     ) {
         // TODO: HANDLE KEYWORDS LATER!
         if (aggregatedStarTreeDocument == null) {
-            long[] dimensions = Arrays.copyOf(segmentStarTreeDocument._dimensions, numDimensions);
+            long[] dimensions = Arrays.copyOf(segmentStarTreeDocument.dimensions, numDimensions);
             Object[] metrics = new Object[numMetrics];
             for (int i = 0; i < numMetrics; i++) {
-                metrics[i] = valueAggregators[i].getInitialAggregatedValue(segmentStarTreeDocument._metrics[i]);
+                metrics[i] = valueAggregators[i].getInitialAggregatedValue(segmentStarTreeDocument.metrics[i]);
             }
             return new StarTreeDocument(dimensions, metrics);
         } else {
             for (int i = 0; i < numMetrics; i++) {
-                aggregatedStarTreeDocument._metrics[i] = valueAggregators[i].applyRawValue(
-                    aggregatedStarTreeDocument._metrics[i],
-                    segmentStarTreeDocument._metrics[i]
+                aggregatedStarTreeDocument.metrics[i] = valueAggregators[i].applyRawValue(
+                    aggregatedStarTreeDocument.metrics[i],
+                    segmentStarTreeDocument.metrics[i]
                 );
             }
             return aggregatedStarTreeDocument;
@@ -324,17 +325,17 @@ protected StarTreeDocument mergeStarTreeDocument(
         StarTreeDocument starTreeStarTreeDocument
     ) {
         if (aggregatedStarTreeDocument == null) {
-            long[] dimensions = Arrays.copyOf(starTreeStarTreeDocument._dimensions, numDimensions);
+            long[] dimensions = Arrays.copyOf(starTreeStarTreeDocument.dimensions, numDimensions);
             Object[] metrics = new Object[numMetrics];
             for (int i = 0; i < numMetrics; i++) {
-                metrics[i] = valueAggregators[i].cloneAggregatedValue((Long) starTreeStarTreeDocument._metrics[i]);
+                metrics[i] = valueAggregators[i].cloneAggregatedValue((Long) starTreeStarTreeDocument.metrics[i]);
             }
             return new StarTreeDocument(dimensions, metrics);
         } else {
             for (int i = 0; i < numMetrics; i++) {
-                aggregatedStarTreeDocument._metrics[i] = valueAggregators[i].applyAggregatedValue(
-                    (Long) starTreeStarTreeDocument._metrics[i],
-                    (Long) aggregatedStarTreeDocument._metrics[i]
+                aggregatedStarTreeDocument.metrics[i] = valueAggregators[i].applyAggregatedValue(
+                    (Long) starTreeStarTreeDocument.metrics[i],
+                    (Long) aggregatedStarTreeDocument.metrics[i]
                 );
             }
             return aggregatedStarTreeDocument;
@@ -502,7 +503,7 @@ private StarTreeDocument createAggregatedDocs(StarTreeBuilderUtils.TreeNode node
                 }
                 assert aggregatedStarTreeDocument != null;
                 for (int i = node.dimensionId + 1; i < numDimensions; i++) {
-                    aggregatedStarTreeDocument._dimensions[i] = STAR_IN_DOC_VALUES_INDEX;
+                    aggregatedStarTreeDocument.dimensions[i] = STAR_IN_DOC_VALUES_INDEX;
                 }
                 node.aggregatedDocId = numDocs;
                 appendToStarTree(aggregatedStarTreeDocument);
@@ -526,7 +527,7 @@ private StarTreeDocument createAggregatedDocs(StarTreeBuilderUtils.TreeNode node
                 }
                 assert aggregatedStarTreeDocument != null;
                 for (int i = node.dimensionId + 1; i < numDimensions; i++) {
-                    aggregatedStarTreeDocument._dimensions[i] = STAR_IN_DOC_VALUES_INDEX;
+                    aggregatedStarTreeDocument.dimensions[i] = STAR_IN_DOC_VALUES_INDEX;
                 }
                 node.aggregatedDocId = numDocs;
                 appendToStarTree(aggregatedStarTreeDocument);
@@ -608,8 +609,8 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer) t
         Map<String, Map<Long, BytesRef>> ordinalToSortedSetDocValueMap = new HashMap<>();
         for (int docId = 0; docId < numDocs; docId++) {
             StarTreeDocument starTreeDocument = getStarTreeDocument(docId);
-            for (int i = 0; i < starTreeDocument._dimensions.length; i++) {
-                long val = starTreeDocument._dimensions[i];
+            for (int i = 0; i < starTreeDocument.dimensions.length; i++) {
+                long val = starTreeDocument.dimensions[i];
                 StarTreeDocValuesWriter starTreeDocValuesWriter = dimensionWriters.get(i);
                 switch (starTreeDocValuesWriter.getDocValuesType()) {
                     case SORTED_SET:
@@ -626,9 +627,9 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer) t
                         throw new IllegalStateException("Unsupported doc values type");
                 }
             }
-            for (int i = 0; i < starTreeDocument._metrics.length; i++) {
+            for (int i = 0; i < starTreeDocument.metrics.length; i++) {
                 try {
-                    Number parse = NumberFieldMapper.NumberType.LONG.parse(starTreeDocument._metrics[i], true);
+                    Number parse = NumberFieldMapper.NumberType.LONG.parse(starTreeDocument.metrics[i], true);
                     StarTreeDocValuesWriter starTreeDocValuesWriter = metricWriters.get(i);
                     ((SortedNumericDocValuesWriter) starTreeDocValuesWriter.getDocValuesWriter()).addValue(docId, parse.longValue());
                 } catch (IllegalArgumentException e) {
@@ -727,21 +728,4 @@ public void close() throws IOException {
         }
     }
 
-    /**
-     * Star tree document
-     */
-    public static class StarTreeDocument {
-        public final long[] _dimensions;
-        public final Object[] _metrics;
-
-        public StarTreeDocument(long[] dimensions, Object[] metrics) {
-            _dimensions = dimensions;
-            _metrics = metrics;
-        }
-
-        @Override
-        public String toString() {
-            return Arrays.toString(_dimensions) + " | " + Arrays.toString(_metrics);
-        }
-    }
 }
diff --git a/...in/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java b/...in/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java
@@ -21,15 +21,15 @@ public class MetricTypeFieldPair implements Comparable<MetricTypeFieldPair> {
     public static final String STAR = "*";
     public static final MetricTypeFieldPair COUNT_STAR = new MetricTypeFieldPair(MetricType.COUNT, STAR);
 
-    private final MetricType functionType;
+    private final MetricType metricType;
     private final String field;
 
     /**
      * Constructor for MetricTypeFieldPair
      */
-    public MetricTypeFieldPair(MetricType functionType, String field) {
-        this.functionType = functionType;
-        if (functionType == MetricType.COUNT) {
+    public MetricTypeFieldPair(MetricType metricType, String field) {
+        this.metricType = metricType;
+        if (metricType == MetricType.COUNT) {
             this.field = STAR;
         } else {
             this.field = field;
@@ -39,8 +39,8 @@ public MetricTypeFieldPair(MetricType functionType, String field) {
     /**
      * @return Metric Type
      */
-    public MetricType getFunctionType() {
-        return functionType;
+    public MetricType getMetricType() {
+        return metricType;
     }
 
     /**
@@ -51,42 +51,42 @@ public String getField() {
     }
 
     /**
-     * @return field name with function type and field
+     * @return field name with metric type and field
      */
     public String toFieldName() {
-        return toFieldName(functionType, field);
+        return toFieldName(metricType, field);
     }
 
     /**
-     * Builds field name with function type and field
+     * Builds field name with metric type and field
      */
-    public static String toFieldName(MetricType functionType, String field) {
-        return functionType.getTypeName() + DELIMITER + field;
+    public static String toFieldName(MetricType metricType, String field) {
+        return metricType.getTypeName() + DELIMITER + field;
     }
 
     /**
      * Builds MetricTypeFieldPair from field name
      */
     public static MetricTypeFieldPair fromFieldName(String fieldName) {
         String[] parts = fieldName.split(DELIMITER, 2);
-        return fromFunctionAndFieldName(parts[0], parts[1]);
+        return fromMetricAndFieldName(parts[0], parts[1]);
     }
 
     /**
-     * Builds MetricTypeFieldPair from function and field name
+     * Builds MetricTypeFieldPair from metric and field name
      */
-    private static MetricTypeFieldPair fromFunctionAndFieldName(String functionName, String fieldName) {
-        MetricType functionType = MetricType.fromTypeName(functionName);
-        if (functionType == MetricType.COUNT) {
+    private static MetricTypeFieldPair fromMetricAndFieldName(String metricName, String fieldName) {
+        MetricType metricType = MetricType.fromTypeName(metricName);
+        if (metricType == MetricType.COUNT) {
             return COUNT_STAR;
         } else {
-            return new MetricTypeFieldPair(functionType, fieldName);
+            return new MetricTypeFieldPair(metricType, fieldName);
         }
     }
 
     @Override
     public int hashCode() {
-        return 31 * functionType.hashCode() + field.hashCode();
+        return 31 * metricType.hashCode() + field.hashCode();
     }
 
     @Override
@@ -96,7 +96,7 @@ public boolean equals(Object obj) {
         }
         if (obj instanceof MetricTypeFieldPair) {
             MetricTypeFieldPair anotherPair = (MetricTypeFieldPair) obj;
-            return functionType == anotherPair.functionType && field.equals(anotherPair.field);
+            return metricType == anotherPair.metricType && field.equals(anotherPair.field);
         }
         return false;
     }
@@ -109,7 +109,7 @@ public String toString() {
     @Override
     public int compareTo(MetricTypeFieldPair other) {
         return Comparator.comparing((MetricTypeFieldPair o) -> o.field)
-            .thenComparing((MetricTypeFieldPair o) -> o.functionType)
+            .thenComparing((MetricTypeFieldPair o) -> o.metricType)
             .compare(this, other);
     }
 }
diff --git a/...ain/java/org/opensearch/index/compositeindex/startree/aggregators/SumValueAggregator.java b/...ain/java/org/opensearch/index/compositeindex/startree/aggregators/SumValueAggregator.java
@@ -8,6 +8,7 @@
 package org.opensearch.index.compositeindex.startree.aggregators;
 
 import org.opensearch.index.compositeindex.MetricType;
+import org.opensearch.index.compositeindex.startree.data.DataType;
 
 /**
  * Sum value aggregator for star tree

diff --git a/...c/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregator.java b/...c/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregator.java
@@ -8,6 +8,7 @@
 package org.opensearch.index.compositeindex.startree.aggregators;
 
 import org.opensearch.index.compositeindex.MetricType;
+import org.opensearch.index.compositeindex.startree.data.DataType;
 
 /**
  * A value aggregator that pre-aggregates on the input values for a specific type of aggregation.

diff --git a/...java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactory.java b/...java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactory.java
@@ -8,6 +8,7 @@
 package org.opensearch.index.compositeindex.startree.aggregators;
 
 import org.opensearch.index.compositeindex.MetricType;
+import org.opensearch.index.compositeindex.startree.data.DataType;
 
 /**
  * Value aggregator factory for a given aggregation type

diff --git a/.../main/java/org/opensearch/index/compositeindex/startree/builder/CompositeFieldWriter.java b/.../main/java/org/opensearch/index/compositeindex/startree/builder/CompositeFieldWriter.java
@@ -8,6 +8,8 @@
 
 package org.opensearch.index.compositeindex.startree.builder;
 
+import org.opensearch.index.compositeindex.startree.data.StarTreeDocValues;
+
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;

diff --git a/...eindex/startree/aggregators/DataType.java → ...ompositeindex/startree/data/DataType.java b/...eindex/startree/aggregators/DataType.java → ...ompositeindex/startree/data/DataType.java
@@ -5,7 +5,7 @@
  * this file be licensed under the Apache-2.0 license or a
  * compatible open source license.
  */
-package org.opensearch.index.compositeindex.startree.aggregators;
+package org.opensearch.index.compositeindex.startree.data;
 
 /**
  * Data type of doc values

diff --git a/...x/startree/builder/StarTreeDocValues.java → ...ndex/startree/data/StarTreeDocValues.java b/...x/startree/builder/StarTreeDocValues.java → ...ndex/startree/data/StarTreeDocValues.java
@@ -5,7 +5,7 @@
  * this file be licensed under the Apache-2.0 license or a
  * compatible open source license.
  */
-package org.opensearch.index.compositeindex.startree.builder;
+package org.opensearch.index.compositeindex.startree.data;
 
 import org.apache.lucene.index.SortedNumericDocValues;
 import org.opensearch.index.compositeindex.startree.node.StarTree;

diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/data/StarTreeDocument.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/data/StarTreeDocument.java
@@ -0,0 +1,29 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.index.compositeindex.startree.data;
+
+import java.util.Arrays;
+
+/**
+ * Star tree document
+ */
+public class StarTreeDocument {
+    public final long[] dimensions;
+    public final Object[] metrics;
+
+    public StarTreeDocument(long[] dimensions, Object[] metrics) {
+        this.dimensions = dimensions;
+        this.metrics = metrics;
+    }
+
+    @Override
+    public String toString() {
+        return Arrays.toString(dimensions) + " | " + Arrays.toString(metrics);
+    }
+}
diff --git a/...va/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPairTests.java b/...va/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPairTests.java
@@ -15,13 +15,13 @@ public class MetricTypeFieldPairTests extends OpenSearchTestCase {
 
     public void testConstructor() {
         MetricTypeFieldPair pair = new MetricTypeFieldPair(MetricType.SUM, "column1");
-        assertEquals(MetricType.SUM, pair.getFunctionType());
+        assertEquals(MetricType.SUM, pair.getMetricType());
         assertEquals("column1", pair.getField());
     }
 
     public void testCountStarConstructor() {
         MetricTypeFieldPair pair = new MetricTypeFieldPair(MetricType.COUNT, "anything");
-        assertEquals(MetricType.COUNT, pair.getFunctionType());
+        assertEquals(MetricType.COUNT, pair.getMetricType());
         assertEquals("*", pair.getField());
     }
 
@@ -32,13 +32,13 @@ public void testToFieldName() {
 
     public void testFromFieldName() {
         MetricTypeFieldPair pair = MetricTypeFieldPair.fromFieldName("max__column3");
-        assertEquals(MetricType.MAX, pair.getFunctionType());
+        assertEquals(MetricType.MAX, pair.getMetricType());
         assertEquals("column3", pair.getField());
     }
 
     public void testCountStarFromFieldName() {
         MetricTypeFieldPair pair = MetricTypeFieldPair.fromFieldName("count__*");
-        assertEquals(MetricType.COUNT, pair.getFunctionType());
+        assertEquals(MetricType.COUNT, pair.getMetricType());
         assertEquals("*", pair.getField());
         assertSame(MetricTypeFieldPair.COUNT_STAR, pair);
     }

diff --git a/...org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactoryTests.java b/...org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactoryTests.java
@@ -9,6 +9,7 @@
 package org.opensearch.index.compositeindex.startree.aggregators;
 
 import org.opensearch.index.compositeindex.MetricType;
+import org.opensearch.index.compositeindex.startree.data.DataType;
 import org.opensearch.test.OpenSearchTestCase;
 
 public class ValueAggregatorFactoryTests extends OpenSearchTestCase {