Skip to content

Commit 9106713

Browse files
retareta
authored
DATETIME_FORMATTER_CACHING_SETTING experimental feature should not default to 'true' (opensearch-project#13532)
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
1 parent 0c1cd75 commit 9106713

File tree

6 files changed

+94
-10
lines changed

6 files changed

+94
-10
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
9797
- Improve the error messages for _stats with closed indices ([#13012](https://github.com/opensearch-project/OpenSearch/pull/13012))
9898
- Ignore BaseRestHandler unconsumed content check as it's always consumed. ([#13290](https://github.com/opensearch-project/OpenSearch/pull/13290))
9999
- Fix mapper_parsing_exception when using flat_object fields with names longer than 11 characters ([#13259](https://github.com/opensearch-project/OpenSearch/pull/13259))
100+
- DATETIME_FORMATTER_CACHING_SETTING experimental feature should not default to 'true' ([#13532](https://github.com/opensearch-project/OpenSearch/pull/13532))
100101

101102
### Security
102103

‎server/src/main/java/org/opensearch/common/util/FeatureFlags.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public class FeatureFlags {
8181

8282
public static final Setting<Boolean> DATETIME_FORMATTER_CACHING_SETTING = Setting.boolSetting(
8383
DATETIME_FORMATTER_CACHING,
84-
true,
84+
false,
8585
Property.NodeScope
8686
);
8787

‎server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java

+3-9
Original file line numberDiff line numberDiff line change
@@ -39,30 +39,24 @@ public void testNonBooleanFeatureFlag() {
3939
assertFalse(FeatureFlags.isEnabled(javaVersionProperty));
4040
}
4141

42-
public void testBooleanFeatureFlagWithDefaultSetToTrue() {
43-
final String testFlag = DATETIME_FORMATTER_CACHING;
44-
assertNotNull(testFlag);
45-
assertTrue(FeatureFlags.isEnabled(testFlag));
46-
}
47-
4842
public void testBooleanFeatureFlagWithDefaultSetToFalse() {
4943
final String testFlag = IDENTITY;
5044
FeatureFlags.initializeFeatureFlags(Settings.EMPTY);
5145
assertNotNull(testFlag);
5246
assertFalse(FeatureFlags.isEnabled(testFlag));
5347
}
5448

55-
public void testBooleanFeatureFlagInitializedWithEmptySettingsAndDefaultSetToTrue() {
49+
public void testBooleanFeatureFlagInitializedWithEmptySettingsAndDefaultSetToFalse() {
5650
final String testFlag = DATETIME_FORMATTER_CACHING;
5751
FeatureFlags.initializeFeatureFlags(Settings.EMPTY);
5852
assertNotNull(testFlag);
59-
assertTrue(FeatureFlags.isEnabled(testFlag));
53+
assertFalse(FeatureFlags.isEnabled(testFlag));
6054
}
6155

6256
public void testInitializeFeatureFlagsWithExperimentalSettings() {
6357
FeatureFlags.initializeFeatureFlags(Settings.builder().put(IDENTITY, true).build());
6458
assertTrue(FeatureFlags.isEnabled(IDENTITY));
65-
assertTrue(FeatureFlags.isEnabled(DATETIME_FORMATTER_CACHING));
59+
assertFalse(FeatureFlags.isEnabled(DATETIME_FORMATTER_CACHING));
6660
assertFalse(FeatureFlags.isEnabled(EXTENSIONS));
6761
// reset FeatureFlags to defaults
6862
FeatureFlags.initializeFeatureFlags(Settings.EMPTY);

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

+18
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.lucene.index.DocValuesType;
3636
import org.apache.lucene.index.IndexableField;
3737
import org.opensearch.common.time.DateFormatter;
38+
import org.opensearch.common.util.FeatureFlags;
3839
import org.opensearch.core.xcontent.XContentBuilder;
3940
import org.opensearch.index.termvectors.TermVectorsService;
4041
import org.opensearch.search.DocValueFormat;
@@ -45,8 +46,10 @@
4546
import java.time.ZonedDateTime;
4647
import java.util.List;
4748

49+
import static org.hamcrest.CoreMatchers.is;
4850
import static org.hamcrest.Matchers.containsString;
4951
import static org.hamcrest.Matchers.notNullValue;
52+
import static org.junit.Assume.assumeThat;
5053

5154
public class DateFieldMapperTests extends MapperTestCase {
5255

@@ -146,7 +149,22 @@ public void testStore() throws Exception {
146149
assertEquals(1457654400000L, storedField.numericValue().longValue());
147150
}
148151

152+
public void testIgnoreMalformedLegacy() throws IOException {
153+
assumeThat("Using legacy datetime format as default", FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING), is(false));
154+
testIgnoreMalformedForValue(
155+
"2016-03-99",
156+
"failed to parse date field [2016-03-99] with format [strict_date_optional_time||epoch_millis]"
157+
);
158+
testIgnoreMalformedForValue("-2147483648", "Invalid value for Year (valid values -999999999 - 999999999): -2147483648");
159+
testIgnoreMalformedForValue("-522000000", "long overflow");
160+
}
161+
149162
public void testIgnoreMalformed() throws IOException {
163+
assumeThat(
164+
"Using experimental datetime format as default",
165+
FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING),
166+
is(true)
167+
);
150168
testIgnoreMalformedForValue(
151169
"2016-03-99",
152170
"failed to parse date field [2016-03-99] with format [strict_date_time_no_millis||strict_date_optional_time||epoch_millis]"

‎server/src/test/java/org/opensearch/index/mapper/RangeFieldMapperTests.java

+26
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.apache.lucene.index.IndexableField;
3838
import org.opensearch.common.CheckedConsumer;
3939
import org.opensearch.common.network.InetAddresses;
40+
import org.opensearch.common.util.FeatureFlags;
4041
import org.opensearch.common.xcontent.XContentFactory;
4142
import org.opensearch.core.xcontent.ToXContent;
4243
import org.opensearch.core.xcontent.XContentBuilder;
@@ -51,8 +52,10 @@
5152
import static org.opensearch.index.query.RangeQueryBuilder.GT_FIELD;
5253
import static org.opensearch.index.query.RangeQueryBuilder.LTE_FIELD;
5354
import static org.opensearch.index.query.RangeQueryBuilder.LT_FIELD;
55+
import static org.hamcrest.CoreMatchers.is;
5456
import static org.hamcrest.Matchers.anyOf;
5557
import static org.hamcrest.Matchers.containsString;
58+
import static org.junit.Assume.assumeThat;
5659

5760
public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase {
5861
private static final String FROM_DATE = "2016-10-31";
@@ -351,7 +354,30 @@ public void testIllegalArguments() throws Exception {
351354
assertThat(e.getMessage(), containsString("should not define a dateTimeFormatter"));
352355
}
353356

357+
public void testSerializeDefaultsLegacy() throws Exception {
358+
assumeThat("Using legacy datetime format as default", FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING), is(false));
359+
360+
for (String type : types()) {
361+
DocumentMapper docMapper = createDocumentMapper(fieldMapping(b -> b.field("type", type)));
362+
RangeFieldMapper mapper = (RangeFieldMapper) docMapper.root().getMapper("field");
363+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
364+
mapper.doXContentBody(builder, true, ToXContent.EMPTY_PARAMS);
365+
String got = builder.endObject().toString();
366+
367+
// if type is date_range we check that the mapper contains the default format and locale
368+
// otherwise it should not contain a locale or format
369+
assertTrue(got, got.contains("\"format\":\"strict_date_optional_time||epoch_millis\"") == type.equals("date_range"));
370+
assertTrue(got, got.contains("\"locale\":" + "\"" + Locale.ROOT + "\"") == type.equals("date_range"));
371+
}
372+
}
373+
354374
public void testSerializeDefaults() throws Exception {
375+
assumeThat(
376+
"Using experimental datetime format as default",
377+
FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING),
378+
is(true)
379+
);
380+
355381
for (String type : types()) {
356382
DocumentMapper docMapper = createDocumentMapper(fieldMapping(b -> b.field("type", type)));
357383
RangeFieldMapper mapper = (RangeFieldMapper) docMapper.root().getMapper("field");

‎server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java

+45
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.opensearch.common.settings.Settings;
5252
import org.opensearch.common.time.DateFormatter;
5353
import org.opensearch.common.util.BigArrays;
54+
import org.opensearch.common.util.FeatureFlags;
5455
import org.opensearch.index.IndexSettings;
5556
import org.opensearch.index.mapper.DateFieldMapper.DateFieldType;
5657
import org.opensearch.index.mapper.RangeFieldMapper.RangeFieldType;
@@ -65,8 +66,10 @@
6566
import java.util.Collections;
6667
import java.util.Map;
6768

69+
import static org.hamcrest.CoreMatchers.is;
6870
import static org.hamcrest.Matchers.containsString;
6971
import static org.hamcrest.Matchers.instanceOf;
72+
import static org.junit.Assume.assumeThat;
7073

7174
public class RangeFieldTypeTests extends FieldTypeTestCase {
7275
RangeType type;
@@ -249,7 +252,49 @@ private QueryShardContext createContext() {
249252
);
250253
}
251254

255+
public void testDateRangeQueryUsingMappingFormatLegacy() {
256+
assumeThat("Using legacy datetime format as default", FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING), is(false));
257+
258+
QueryShardContext context = createContext();
259+
RangeFieldType strict = new RangeFieldType("field", RangeFieldMapper.Defaults.DATE_FORMATTER);
260+
// don't use DISJOINT here because it doesn't work on date fields which we want to compare bounds with
261+
ShapeRelation relation = randomValueOtherThan(ShapeRelation.DISJOINT, () -> randomFrom(ShapeRelation.values()));
262+
263+
// dates will break the default format, month/day of month is turned around in the format
264+
final String from = "2016-15-06T15:29:50+08:00";
265+
final String to = "2016-16-06T15:29:50+08:00";
266+
267+
OpenSearchParseException ex = expectThrows(
268+
OpenSearchParseException.class,
269+
() -> strict.rangeQuery(from, to, true, true, relation, null, null, context)
270+
);
271+
assertThat(
272+
ex.getMessage(),
273+
containsString("failed to parse date field [2016-15-06T15:29:50+08:00] with format [strict_date_optional_time||epoch_millis]")
274+
);
275+
276+
// setting mapping format which is compatible with those dates
277+
final DateFormatter formatter = DateFormatter.forPattern("yyyy-dd-MM'T'HH:mm:ssZZZZZ");
278+
assertEquals(1465975790000L, formatter.parseMillis(from));
279+
assertEquals(1466062190000L, formatter.parseMillis(to));
280+
281+
RangeFieldType fieldType = new RangeFieldType("field", formatter);
282+
final Query query = fieldType.rangeQuery(from, to, true, true, relation, null, fieldType.dateMathParser(), context);
283+
assertEquals("field:<ranges:[1465975790000 : 1466062190999]>", ((IndexOrDocValuesQuery) query).getIndexQuery().toString());
284+
285+
// compare lower and upper bounds with what we would get on a `date` field
286+
DateFieldType dateFieldType = new DateFieldType("field", DateFieldMapper.Resolution.MILLISECONDS, formatter);
287+
final Query queryOnDateField = dateFieldType.rangeQuery(from, to, true, true, relation, null, fieldType.dateMathParser(), context);
288+
assertEquals("field:[1465975790000 TO 1466062190999]", ((IndexOrDocValuesQuery) queryOnDateField).getIndexQuery().toString());
289+
}
290+
252291
public void testDateRangeQueryUsingMappingFormat() {
292+
assumeThat(
293+
"Using experimental datetime format as default",
294+
FeatureFlags.isEnabled(FeatureFlags.DATETIME_FORMATTER_CACHING),
295+
is(true)
296+
);
297+
253298
QueryShardContext context = createContext();
254299
RangeFieldType strict = new RangeFieldType("field", RangeFieldMapper.Defaults.DATE_FORMATTER);
255300
// don't use DISJOINT here because it doesn't work on date fields which we want to compare bounds with

0 commit comments

Comments
 (0)