Skip to content

Commit 08ed9ee

Browse files
authored
Enable skip_list for @timestamp field or index sort field (#19480)
Signed-off-by: Asim Mahmood <asim.seng@gmail.com>
1 parent 2817029 commit 08ed9ee

File tree

4 files changed

+57
-2
lines changed

4 files changed

+57
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3030
- Add all-active ingestion as docrep equivalent in pull-based ingestion ([#19316](https://github.com/opensearch-project/OpenSearch/pull/19316))
3131
- Adding logic for histogram aggregation using skiplist ([#19130](https://github.com/opensearch-project/OpenSearch/pull/19130))
3232
- Add skip_list param for date, scaled float and token count fields ([#19142](https://github.com/opensearch-project/OpenSearch/pull/19142))
33+
- Enable skip_list for @timestamp field or index sort field by default([#19480](https://github.com/opensearch-project/OpenSearch/pull/19480))
3334
- Implement GRPC MatchPhrase, MultiMatch queries ([#19449](https://github.com/opensearch-project/OpenSearch/pull/19449))
3435
- Optimize gRPC transport thread management for improved throughput ([#19278](https://github.com/opensearch-project/OpenSearch/pull/19278))
3536
- Implement GRPC Boolean query and inject registry for all internal query converters ([#19391](https://github.com/opensearch-project/OpenSearch/pull/19391))

server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.opensearch.common.util.FeatureFlags;
5959
import org.opensearch.common.util.LocaleUtils;
6060
import org.opensearch.common.xcontent.support.XContentMapValues;
61+
import org.opensearch.index.IndexSortConfig;
6162
import org.opensearch.index.compositeindex.datacube.DimensionType;
6263
import org.opensearch.index.fielddata.IndexFieldData;
6364
import org.opensearch.index.fielddata.IndexNumericFieldData.NumericType;
@@ -753,6 +754,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
753754
private final boolean indexed;
754755
private final boolean hasDocValues;
755756
private final boolean skiplist;
757+
private final boolean isSkiplistConfigured;
756758
private final Locale locale;
757759
private final String format;
758760
private final String printFormat;
@@ -778,6 +780,7 @@ private DateFieldMapper(
778780
this.indexed = builder.index.getValue();
779781
this.hasDocValues = builder.docValues.getValue();
780782
this.skiplist = builder.skiplist.getValue();
783+
this.isSkiplistConfigured = builder.skiplist.isConfigured();
781784
this.locale = builder.locale.getValue();
782785
this.format = builder.format.getValue();
783786
this.printFormat = builder.printFormat.getValue();
@@ -846,7 +849,7 @@ protected void parseCreateField(ParseContext context) throws IOException {
846849
context.doc().add(new LongPoint(fieldType().name(), timestamp));
847850
}
848851
if (hasDocValues) {
849-
if (skiplist) {
852+
if (skiplist || isSkiplistDefaultEnabled(context.indexSettings().getIndexSortConfig(), fieldType().name())) {
850853
context.doc().add(SortedNumericDocValuesField.indexedField(fieldType().name(), timestamp));
851854
} else {
852855
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), timestamp));
@@ -859,6 +862,19 @@ protected void parseCreateField(ParseContext context) throws IOException {
859862
}
860863
}
861864

865+
boolean isSkiplistDefaultEnabled(IndexSortConfig indexSortConfig, String fieldName) {
866+
if (!isSkiplistConfigured) {
867+
if (indexSortConfig.hasPrimarySortOnField(fieldName)) {
868+
return true;
869+
}
870+
if (DataStreamFieldMapper.Defaults.TIMESTAMP_FIELD.getName().equals(fieldName)) {
871+
return true;
872+
}
873+
874+
}
875+
return false;
876+
}
877+
862878
public Long getNullValue() {
863879
return nullValue;
864880
}

server/src/test/java/org/opensearch/index/mapper/DateFieldMapperTests.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,15 @@
4444
import org.apache.lucene.index.StoredFields;
4545
import org.apache.lucene.index.VectorEncoding;
4646
import org.apache.lucene.index.VectorSimilarityFunction;
47+
import org.opensearch.Version;
48+
import org.opensearch.cluster.metadata.IndexMetadata;
49+
import org.opensearch.common.settings.Settings;
4750
import org.opensearch.common.time.DateFormatter;
4851
import org.opensearch.common.util.FeatureFlags;
4952
import org.opensearch.common.xcontent.XContentFactory;
5053
import org.opensearch.core.xcontent.XContentBuilder;
54+
import org.opensearch.index.IndexSettings;
55+
import org.opensearch.index.IndexSortConfig;
5156
import org.opensearch.index.fieldvisitor.SingleFieldsVisitor;
5257
import org.opensearch.index.termvectors.TermVectorsService;
5358
import org.opensearch.search.DocValueFormat;
@@ -833,6 +838,35 @@ public void testSkipListIntegrationMappingDefinitionSerialization() throws IOExc
833838
assertTrue("skip_list should be true in date_nanos mapper", dateFieldMapperNanos.skiplist());
834839
}
835840

841+
public void testIsSkiplistDefaultEnabled() throws IOException {
842+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date")));
843+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
844+
845+
// Test with no index sort and non-timestamp field
846+
IndexMetadata noSortindexMetadata = new IndexMetadata.Builder("index").settings(getIndexSettings()).build();
847+
IndexSettings noSolrIndexSettings = new IndexSettings(noSortindexMetadata, getIndexSettings());
848+
IndexSortConfig noSortConfig = new IndexSortConfig(new IndexSettings(noSortindexMetadata, getIndexSettings()));
849+
assertFalse(dateFieldMapper.isSkiplistDefaultEnabled(noSortConfig, "field"));
850+
851+
// timestamp field
852+
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(noSortConfig, "@timestamp"));
853+
854+
// Create index settings with an index sort.
855+
Settings settings = Settings.builder()
856+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
857+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
858+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
859+
.putList("index.sort.field", "field")
860+
.build();
861+
862+
// Test with timestamp field
863+
IndexMetadata indexMetadata = new IndexMetadata.Builder("index").settings(settings).build();
864+
IndexSettings indexSettings = new IndexSettings(indexMetadata, settings);
865+
IndexSortConfig sortConfig = new IndexSortConfig(indexSettings);
866+
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(sortConfig, "field"));
867+
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(sortConfig, "@timestamp"));
868+
}
869+
836870
public void testSkipListIntegrationFieldBehaviorConsistency() throws IOException {
837871
// Test that field behavior is consistent between skip_list enabled and disabled
838872

test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@
8080

8181
public abstract class MapperServiceTestCase extends OpenSearchTestCase {
8282

83-
protected static final Settings SETTINGS = Settings.builder().put("index.version.created", Version.CURRENT).build();
83+
protected static final Settings SETTINGS = Settings.builder()
84+
.put("index.version.created", Version.CURRENT)
85+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
86+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
87+
.build();
8488

8589
protected static final ToXContent.Params INCLUDE_DEFAULTS = new ToXContent.MapParams(
8690
Collections.singletonMap("include_defaults", "true")

0 commit comments

Comments
 (0)