From c135dff6a8039b66edc86cd775b7c536bbbf6231 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Mon, 10 Jun 2024 18:37:12 +0530 Subject: [PATCH] Refactoring Signed-off-by: Sarthak Aggarwal --- .../BaseCompositeFieldStarTreeBuilder.java | 54 +++++++------------ .../aggregators/MetricTypeFieldPair.java | 40 +++++++------- .../aggregators/SumValueAggregator.java | 1 + .../startree/aggregators/ValueAggregator.java | 1 + .../aggregators/ValueAggregatorFactory.java | 1 + .../builder/CompositeFieldWriter.java | 2 + .../{aggregators => data}/DataType.java | 2 +- .../{builder => data}/StarTreeDocValues.java | 2 +- .../startree/data/StarTreeDocument.java | 29 ++++++++++ .../aggregators/MetricTypeFieldPairTests.java | 8 +-- .../ValueAggregatorFactoryTests.java | 1 + 11 files changed, 80 insertions(+), 61 deletions(-) rename server/src/main/java/org/opensearch/index/compositeindex/startree/{aggregators => data}/DataType.java (96%) rename server/src/main/java/org/opensearch/index/compositeindex/startree/{builder => data}/StarTreeDocValues.java (94%) create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/startree/data/StarTreeDocument.java diff --git a/server/src/main/java/org/apache/lucene/index/BaseCompositeFieldStarTreeBuilder.java b/server/src/main/java/org/apache/lucene/index/BaseCompositeFieldStarTreeBuilder.java index 4c78976c0d4a7..a62a9ee38608b 100644 --- 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> 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/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java index f5e39c0610119..ca7caaa5535ee 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPair.java @@ -21,15 +21,15 @@ public class MetricTypeFieldPair implements Comparable { 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,17 +51,17 @@ 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; } /** @@ -69,24 +69,24 @@ public static String toFieldName(MetricType functionType, String field) { */ 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/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/SumValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/SumValueAggregator.java index c3bf11f1775fa..9fbfdd26df57c 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/SumValueAggregator.java +++ b/server/src/main/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/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregator.java index cc3410f3e3e7e..bfbbc05254d5e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregator.java +++ b/server/src/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/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactory.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactory.java index 20a054b30f64d..bb1df58ca6b2b 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactory.java +++ b/server/src/main/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/server/src/main/java/org/opensearch/index/compositeindex/startree/builder/CompositeFieldWriter.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/builder/CompositeFieldWriter.java index 84a197fdcf1df..7abd10f6e9409 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/builder/CompositeFieldWriter.java +++ b/server/src/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/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/DataType.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/data/DataType.java similarity index 96% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/DataType.java rename to server/src/main/java/org/opensearch/index/compositeindex/startree/data/DataType.java index 5dc04d4196659..c15697d722563 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/aggregators/DataType.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/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/server/src/main/java/org/opensearch/index/compositeindex/startree/builder/StarTreeDocValues.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/data/StarTreeDocValues.java similarity index 94% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/builder/StarTreeDocValues.java rename to server/src/main/java/org/opensearch/index/compositeindex/startree/data/StarTreeDocValues.java index abebd21444cc4..bdedcefb97a61 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/builder/StarTreeDocValues.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/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 new file mode 100644 index 0000000000000..44b211123c19f --- /dev/null +++ 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/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPairTests.java b/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPairTests.java index 0827d86162754..1dca82708a19c 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/MetricTypeFieldPairTests.java +++ b/server/src/test/java/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/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactoryTests.java b/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactoryTests.java index 60129e0a7dc96..3ed8d0a8858e9 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/startree/aggregators/ValueAggregatorFactoryTests.java +++ b/server/src/test/java/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 {