Skip to content

Commit b3453b9

Browse files
committed
Revert "Enable skip_list for @timestamp field or index sort field (#19480)"
This reverts commit 08ed9ee. Fixes #19660 Signed-off-by: Asim Mahmood <[email protected]>
1 parent b9781c1 commit b3453b9

File tree

4 files changed

+4
-56
lines changed

4 files changed

+4
-56
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2626
- Avoid primary shard failure caused by merged segment warmer exceptions ([#19436](https://github.com/opensearch-project/OpenSearch/pull/19436))
2727
- Fix pull-based ingestion out-of-bounds offset scenarios and remove persisted offsets ([#19607](https://github.com/opensearch-project/OpenSearch/pull/19607))
2828
- [Star Tree] Fix sub-aggregator casting for search with profile=true ([19652](https://github.com/opensearch-project/OpenSearch/pull/19652))
29+
- Remvoe default enable skip_list for @timestamp field or index sort field to fix upgrades ([TBD](TBD))
30+
- Remove default enable skip_list for @timestamp field or index sort field to fix upgrades ([19611](https://github.com/opensearch-project/OpenSearch/pull/19661))
2931

3032
### Dependencies
3133
- Update to Gradle 9.1 ([#19575](https://github.com/opensearch-project/OpenSearch/pull/19575))

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

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
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;
6261
import org.opensearch.index.compositeindex.datacube.DimensionType;
6362
import org.opensearch.index.fielddata.IndexFieldData;
6463
import org.opensearch.index.fielddata.IndexNumericFieldData.NumericType;
@@ -754,7 +753,6 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
754753
private final boolean indexed;
755754
private final boolean hasDocValues;
756755
private final boolean skiplist;
757-
private final boolean isSkiplistConfigured;
758756
private final Locale locale;
759757
private final String format;
760758
private final String printFormat;
@@ -780,7 +778,6 @@ private DateFieldMapper(
780778
this.indexed = builder.index.getValue();
781779
this.hasDocValues = builder.docValues.getValue();
782780
this.skiplist = builder.skiplist.getValue();
783-
this.isSkiplistConfigured = builder.skiplist.isConfigured();
784781
this.locale = builder.locale.getValue();
785782
this.format = builder.format.getValue();
786783
this.printFormat = builder.printFormat.getValue();
@@ -838,7 +835,7 @@ protected void parseCreateField(ParseContext context) throws IOException {
838835
context.doc().add(new LongPoint(fieldType().name(), timestamp));
839836
}
840837
if (hasDocValues) {
841-
if (skiplist || isSkiplistDefaultEnabled(context.indexSettings().getIndexSortConfig(), fieldType().name())) {
838+
if (skiplist) {
842839
context.doc().add(SortedNumericDocValuesField.indexedField(fieldType().name(), timestamp));
843840
} else {
844841
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), timestamp));
@@ -851,19 +848,6 @@ protected void parseCreateField(ParseContext context) throws IOException {
851848
}
852849
}
853850

854-
boolean isSkiplistDefaultEnabled(IndexSortConfig indexSortConfig, String fieldName) {
855-
if (!isSkiplistConfigured) {
856-
if (indexSortConfig.hasPrimarySortOnField(fieldName)) {
857-
return true;
858-
}
859-
if (DataStreamFieldMapper.Defaults.TIMESTAMP_FIELD.getName().equals(fieldName)) {
860-
return true;
861-
}
862-
863-
}
864-
return false;
865-
}
866-
867851
@Override
868852
protected String getFieldValue(ParseContext context) throws IOException {
869853
if (context.externalValueSet()) {

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

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,10 @@
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;
5047
import org.opensearch.common.time.DateFormatter;
5148
import org.opensearch.common.util.FeatureFlags;
5249
import org.opensearch.common.xcontent.XContentFactory;
5350
import org.opensearch.core.xcontent.XContentBuilder;
54-
import org.opensearch.index.IndexSettings;
55-
import org.opensearch.index.IndexSortConfig;
5651
import org.opensearch.index.fieldvisitor.SingleFieldsVisitor;
5752
import org.opensearch.index.termvectors.TermVectorsService;
5853
import org.opensearch.search.DocValueFormat;
@@ -867,35 +862,6 @@ public void testSkipListIntegrationMappingDefinitionSerialization() throws IOExc
867862
assertTrue("skip_list should be true in date_nanos mapper", dateFieldMapperNanos.skiplist());
868863
}
869864

870-
public void testIsSkiplistDefaultEnabled() throws IOException {
871-
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date")));
872-
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
873-
874-
// Test with no index sort and non-timestamp field
875-
IndexMetadata noSortindexMetadata = new IndexMetadata.Builder("index").settings(getIndexSettings()).build();
876-
IndexSettings noSolrIndexSettings = new IndexSettings(noSortindexMetadata, getIndexSettings());
877-
IndexSortConfig noSortConfig = new IndexSortConfig(new IndexSettings(noSortindexMetadata, getIndexSettings()));
878-
assertFalse(dateFieldMapper.isSkiplistDefaultEnabled(noSortConfig, "field"));
879-
880-
// timestamp field
881-
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(noSortConfig, "@timestamp"));
882-
883-
// Create index settings with an index sort.
884-
Settings settings = Settings.builder()
885-
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
886-
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
887-
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
888-
.putList("index.sort.field", "field")
889-
.build();
890-
891-
// Test with timestamp field
892-
IndexMetadata indexMetadata = new IndexMetadata.Builder("index").settings(settings).build();
893-
IndexSettings indexSettings = new IndexSettings(indexMetadata, settings);
894-
IndexSortConfig sortConfig = new IndexSortConfig(indexSettings);
895-
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(sortConfig, "field"));
896-
assertTrue(dateFieldMapper.isSkiplistDefaultEnabled(sortConfig, "@timestamp"));
897-
}
898-
899865
public void testSkipListIntegrationFieldBehaviorConsistency() throws IOException {
900866
// Test that field behavior is consistent between skip_list enabled and disabled
901867

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

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

8181
public abstract class MapperServiceTestCase extends OpenSearchTestCase {
8282

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();
83+
protected static final Settings SETTINGS = Settings.builder().put("index.version.created", Version.CURRENT).build();
8884

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

0 commit comments

Comments
 (0)