Skip to content

Commit

Permalink
Adding support for date dim and adding back tests resolving conflicts
Browse files Browse the repository at this point in the history 
Signed-off-by: Bharathwaj G <[email protected]>
  • Loading branch information
@bharath-techie
bharath-techie committed Sep 9, 2024
1 parent 7243dd1 commit 9a4e7f3
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 58 deletions.
45 changes: 23 additions & 22 deletions server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.opensearch.action.index.IndexResponse;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.action.support.master.AcknowledgedResponse;
import org.opensearch.common.Rounding;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.common.unit.ByteSizeUnit;
Expand All @@ -22,6 +23,7 @@
import org.opensearch.index.IndexService;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.compositeindex.CompositeIndexSettings;
import org.opensearch.index.compositeindex.datacube.DateDimension;
import org.opensearch.index.compositeindex.datacube.MetricStat;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
Expand Down Expand Up @@ -62,10 +64,7 @@ private static XContentBuilder createMinimalTestMapping(boolean invalidDim, bool
.field("type", "star_tree")
.startObject("config")
.startObject("date_dimension")
.field("name", "numeric_dv_1")
.endObject()
.startObject()
.field("name", "numeric_dv_2")
.field("name", "timestamp")
.endObject()
.startArray("ordered_dimensions")
.startObject()
Expand All @@ -88,14 +87,6 @@ private static XContentBuilder createMinimalTestMapping(boolean invalidDim, bool
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric_dv_1")
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric_dv_2")
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric")
.field("type", "integer")
.field("doc_values", false)
Expand Down Expand Up @@ -268,7 +259,7 @@ private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolea
.field("type", "star_tree")
.startObject("config")
.startObject("date_dimension")
.field("name", "numeric_dv1")
.field("name", "timestamp")
.endObject()
.startArray("ordered_dimensions")
.startObject()
Expand All @@ -291,10 +282,6 @@ private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolea
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric_dv1")
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric")
.field("type", "integer")
.field("doc_values", false)
Expand Down Expand Up @@ -327,7 +314,7 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo
.field("type", "star_tree")
.startObject("config")
.startObject("date_dimension")
.field("name", "numeric_dv2")
.field("name", "timestamp")
.endObject()
.startArray("ordered_dimensions")
.startObject()
Expand Down Expand Up @@ -356,10 +343,6 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric_dv2")
.field("type", "integer")
.field("doc_values", true)
.endObject()
.startObject("numeric_dv1")
.field("type", "integer")
.field("doc_values", true)
Expand Down Expand Up @@ -727,6 +710,24 @@ public void testMaxMetricsCompositeIndex() {
);
}

public void testMaxCalendarIntervalsCompositeIndex() {
MapperParsingException ex = expectThrows(
MapperParsingException.class,
() -> prepareCreate(TEST_INDEX).setMapping(createMaxDimTestMapping())
.setSettings(
Settings.builder()
.put(StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.getKey(), 1)
.put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true)
.put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB))
)
.get()
);
assertEquals(
"Failed to parse mapping [_doc]: At most [1] calendar intervals are allowed in dimension [timestamp]",
ex.getMessage()
);
}

public void testUnsupportedDim() {
MapperParsingException ex = expectThrows(
MapperParsingException.class,
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ protected void setReadersAndNumSegmentDocs(
}
// get doc id set iterators for metrics
for (Metric metric : starTreeValues.getStarTreeField().getMetrics()) {
for (MetricStat metricStat : metric.getMetrics()) {
for (MetricStat metricStat : metric.getBaseMetrics()) {
String metricFullName = fullyQualifiedFieldNameForStarTreeMetricsDocValues(
starTreeValues.getStarTreeField().getName(),
metric.getField(),
Expand Down
5 changes: 0 additions & 5 deletions ...main/java/org/opensearch/index/compositeindex/datacube/startree/index/StarTreeValues.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
Expand Down Expand Up @@ -249,8 +248,6 @@ public DocIdSetIterator getDimensionDocIdSetIterator(String dimension) {

if (dimensionDocValuesIteratorMap.containsKey(dimension)) {
return dimensionDocValuesIteratorMap.get(dimension).get();
} else {
throw new IllegalArgumentException(String.format(Locale.ROOT, "Dimension %s not present", dimension));
}

throw new IllegalArgumentException("dimension [" + dimension + "] does not exist in the segment.");
Expand All @@ -266,8 +263,6 @@ public DocIdSetIterator getMetricDocIdSetIterator(String fullyQualifiedMetricNam

if (metricDocValuesIteratorMap.containsKey(fullyQualifiedMetricName)) {
return metricDocValuesIteratorMap.get(fullyQualifiedMetricName).get();
} else {
throw new IllegalArgumentException(String.format(Locale.ROOT, "Metric %s not present", fullyQualifiedMetricName));
}

throw new IllegalArgumentException("metric [" + fullyQualifiedMetricName + "] does not exist in the segment.");
Expand Down
6 changes: 6 additions & 0 deletions server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.common.util.LocaleUtils;
import org.opensearch.index.compositeindex.datacube.DimensionType;
import org.opensearch.index.fielddata.IndexFieldData;
import org.opensearch.index.fielddata.IndexNumericFieldData.NumericType;
import org.opensearch.index.fielddata.plain.SortedNumericIndexFieldData;
Expand All @@ -76,6 +77,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.LongSupplier;
Expand Down Expand Up @@ -350,6 +352,10 @@ public DateFieldMapper build(BuilderContext context) {
return new DateFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), nullTimestamp, resolution, this);
}

@Override
public Optional<DimensionType> getSupportedDataCubeDimensionType() {
return Optional.of(DimensionType.DATE);
}
}

public static final TypeParser MILLIS_PARSER = new TypeParser((n, c) -> {
Expand Down
5 changes: 3 additions & 2 deletions ...ensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4186,7 +4186,8 @@ public void testMergeFlow() throws IOException {
builder.appendDocumentsToStarTree(starTreeDocumentIterator);
for (StarTreeDocument starTreeDocument : builder.getStarTreeDocuments()) {
assertEquals(starTreeDocument.dimensions[0] * 20.0, starTreeDocument.metrics[0]);
assertEquals(2L, starTreeDocument.metrics[1]);
assertEquals(starTreeDocument.dimensions[0] * 2, starTreeDocument.metrics[1]);
assertEquals(2L, starTreeDocument.metrics[2]);
}
builder.build(starTreeDocumentIterator, new AtomicInteger(), docValuesConsumer);

Expand Down Expand Up @@ -4443,7 +4444,7 @@ private StarTreeValues getStarTreeValuesWithDates(
),
() -> metricsList1
);
return new StarTreeValues(sf, null, dimDocIdSetIterators, metricDocIdSetIterators, Map.of(SEGMENT_DOCS_COUNT, number));
return new StarTreeValues(sf, null, dimDocIdSetIterators, metricDocIdSetIterators, Map.of(SEGMENT_DOCS_COUNT, number), null);
}

private StarTreeField getStarTreeFieldWithDateDimension() {
Expand Down
68 changes: 40 additions & 28 deletions server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void testValidStarTree() throws IOException {
assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs());
assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode());
assertEquals(
new HashSet<>(Arrays.asList("node", "status")),
new HashSet<>(Arrays.asList("@timestamp", "status")),
starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()
);
}
Expand Down Expand Up @@ -162,7 +162,7 @@ public void testMetricsWithJustSum() throws IOException {
assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs());
assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode());
assertEquals(
new HashSet<>(Arrays.asList("node", "status")),
new HashSet<>(Arrays.asList("@timestamp", "status")),
starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()
);
}
Expand Down Expand Up @@ -197,7 +197,7 @@ public void testMetricsWithCountAndSum() throws IOException {
assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs());
assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode());
assertEquals(
new HashSet<>(Arrays.asList("node", "status")),
new HashSet<>(Arrays.asList("@timestamp", "status")),
starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()
);
}
Expand Down Expand Up @@ -612,7 +612,7 @@ public void testValidations() throws IOException {
)
);
assertEquals(
"Aggregations not supported for the dimension field [node] with field type [integer] as part of star tree field",
"Aggregations not supported for the dimension field [@timestamp] with field type [date] as part of star tree field",
ex.getMessage()
);

Expand All @@ -635,12 +635,16 @@ private XContentBuilder getExpandedMappingWithJustAvg(String dim, String metric)
b.field("max_leaf_docs", 100);
b.startArray("skip_star_node_creation_for_dimensions");
{
b.value("node");
b.value("@timestamp");
b.value("status");
}
b.endArray();
b.startObject("date_dimension");
b.field("name", "node");
b.field("name", "@timestamp");
b.startArray("calendar_intervals");
b.value("day");
b.value("month");
b.endArray();
b.endObject();
b.startArray("ordered_dimensions");
b.startObject();
Expand All @@ -659,8 +663,8 @@ private XContentBuilder getExpandedMappingWithJustAvg(String dim, String metric)
b.endObject();
b.endObject();
b.startObject("properties");
b.startObject("node");
b.field("type", "integer");
b.startObject("@timestamp");
b.field("type", "date");
b.endObject();
b.startObject("status");
b.field("type", "integer");
Expand All @@ -681,7 +685,7 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo
.field("type", "star_tree")
.startObject("config")
.startObject("date_dimension")
.field("name", "node")
.field("name", "timestamp")
.endObject()
.startArray("ordered_dimensions")
.startObject()
Expand All @@ -703,8 +707,8 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo
.endObject()
.endObject()
.startObject("properties")
.startObject("node")
.field("type", "integer")
.startObject("timestamp")
.field("type", "date")
.endObject()
.startObject("numeric_dv")
.field("type", "integer")
Expand All @@ -731,12 +735,16 @@ private XContentBuilder getExpandedMappingWithJustSum(String dim, String metric)
b.field("max_leaf_docs", 100);
b.startArray("skip_star_node_creation_for_dimensions");
{
b.value("node");
b.value("@timestamp");
b.value("status");
}
b.endArray();
b.startObject("date_dimension");
b.field("name", "node");
b.field("name", "@timestamp");
b.startArray("calendar_intervals");
b.value("day");
b.value("month");
b.endArray();
b.endObject();
b.startArray("ordered_dimensions");
b.startObject();
Expand All @@ -755,8 +763,8 @@ private XContentBuilder getExpandedMappingWithJustSum(String dim, String metric)
b.endObject();
b.endObject();
b.startObject("properties");
b.startObject("node");
b.field("type", "integer");
b.startObject("@timestamp");
b.field("type", "date");
b.endObject();
b.startObject("status");
b.field("type", "integer");
Expand All @@ -777,12 +785,16 @@ private XContentBuilder getExpandedMappingWithSumAndCount(String dim, String met
b.field("max_leaf_docs", 100);
b.startArray("skip_star_node_creation_for_dimensions");
{
b.value("node");
b.value("@timestamp");
b.value("status");
}
b.endArray();
b.startObject("date_dimension");
b.field("name", "node");
b.field("name", "@timestamp");
b.startArray("calendar_intervals");
b.value("day");
b.value("month");
b.endArray();
b.endObject();
b.startArray("ordered_dimensions");
b.startObject();
Expand All @@ -802,8 +814,8 @@ private XContentBuilder getExpandedMappingWithSumAndCount(String dim, String met
b.endObject();
b.endObject();
b.startObject("properties");
b.startObject("node");
b.field("type", "integer");
b.startObject("@timestamp");
b.field("type", "date");
b.endObject();
b.startObject("status");
b.field("type", "integer");
Expand Down Expand Up @@ -902,7 +914,7 @@ private XContentBuilder getMinMapping(
b.startObject("config");
if (!isEmptyDims) {
b.startObject("date_dimension");
b.field("name", "node");
b.field("name", "@timestamp");
b.endObject();
b.startArray("ordered_dimensions");
b.startObject();
Expand All @@ -925,8 +937,8 @@ private XContentBuilder getMinMapping(
b.endObject();
b.startObject("properties");
if (!missingDateDim) {
b.startObject("node");
b.field("type", "integer");
b.startObject("@timestamp");
b.field("type", "date");
b.endObject();
}
if (!missingDim) {
Expand Down Expand Up @@ -1066,9 +1078,9 @@ private XContentBuilder getInvalidMapping(
b.endObject();
b.endObject();
b.startObject("properties");
b.startObject("node");
b.startObject("@timestamp");
if (!invalidDate) {
b.field("type", "integer");
b.field("type", "date");
} else {
b.field("type", "keyword");
}
Expand Down Expand Up @@ -1113,7 +1125,7 @@ private XContentBuilder getInvalidMappingWithDv(
}
b.endArray();
b.startObject("date_dimension");
b.field("name", "node");
b.field("name", "@timestamp");
b.endObject();
b.startArray("ordered_dimensions");
if (!singleDim) {
Expand All @@ -1134,12 +1146,12 @@ private XContentBuilder getInvalidMappingWithDv(
b.endObject();
b.endObject();
b.startObject("properties");
b.startObject("node");
b.startObject("@timestamp");
if (!invalidDimType) {
b.field("type", "integer");
b.field("type", "date");
b.field("doc_values", "true");
} else {
b.field("type", "integer");
b.field("type", "date");
b.field("doc_values", "false");
}
b.endObject();
Expand Down

0 comments on commit 9a4e7f3

Please sign in to comment.