Skip to content

Commit 4ea18ff

Browse files
asimmahmood1asimmahmood1
committed
Add skiplist to datefieldmapper
Signed-off-by: Asim Mahmood <asim.seng@gmail.com>
1 parent 64cae12 commit 4ea18ff

File tree

2 files changed

+241
-2
lines changed

2 files changed

+241
-2
lines changed

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.opensearch.common.unit.TimeValue;
5858
import org.opensearch.common.util.FeatureFlags;
5959
import org.opensearch.common.util.LocaleUtils;
60+
import org.opensearch.common.xcontent.support.XContentMapValues;
6061
import org.opensearch.index.compositeindex.datacube.DimensionType;
6162
import org.opensearch.index.fielddata.IndexFieldData;
6263
import org.opensearch.index.fielddata.IndexNumericFieldData.NumericType;
@@ -269,6 +270,13 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
269270
private final Parameter<Boolean> index = Parameter.indexParam(m -> toType(m).indexed, true);
270271
private final Parameter<Boolean> docValues = Parameter.docValuesParam(m -> toType(m).hasDocValues, true);
271272
private final Parameter<Boolean> store = Parameter.storeParam(m -> toType(m).store, false);
273+
private final Parameter<Boolean> skiplist = new Parameter<>(
274+
"skip_list",
275+
false,
276+
() -> false,
277+
(n, c, o) -> XContentMapValues.nodeBooleanValue(o),
278+
m -> toType(m).skiplist
279+
);
272280

273281
private final Parameter<Float> boost = Parameter.boostParam();
274282
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
@@ -337,7 +345,7 @@ private DateFormatter buildFormatter() {
337345

338346
@Override
339347
protected List<Parameter<?>> getParameters() {
340-
return Arrays.asList(index, docValues, store, format, printFormat, locale, nullValue, ignoreMalformed, boost, meta);
348+
return Arrays.asList(index, docValues, store, skiplist, format, printFormat, locale, nullValue, ignoreMalformed, boost, meta);
341349
}
342350

343351
private Long parseNullValue(DateFieldType fieldType) {
@@ -744,6 +752,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
744752
private final boolean store;
745753
private final boolean indexed;
746754
private final boolean hasDocValues;
755+
private final boolean skiplist;
747756
private final Locale locale;
748757
private final String format;
749758
private final String printFormat;
@@ -768,6 +777,7 @@ private DateFieldMapper(
768777
this.store = builder.store.getValue();
769778
this.indexed = builder.index.getValue();
770779
this.hasDocValues = builder.docValues.getValue();
780+
this.skiplist = builder.skiplist.getValue();
771781
this.locale = builder.locale.getValue();
772782
this.format = builder.format.getValue();
773783
this.printFormat = builder.printFormat.getValue();
@@ -836,7 +846,11 @@ protected void parseCreateField(ParseContext context) throws IOException {
836846
context.doc().add(new LongPoint(fieldType().name(), timestamp));
837847
}
838848
if (hasDocValues) {
839-
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), timestamp));
849+
if (skiplist) {
850+
context.doc().add(SortedNumericDocValuesField.indexedField(fieldType().name(), timestamp));
851+
} else {
852+
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), timestamp));
853+
}
840854
} else if (store || indexed) {
841855
createFieldNamesField(context);
842856
}
@@ -849,6 +863,10 @@ public Long getNullValue() {
849863
return nullValue;
850864
}
851865

866+
public boolean skiplist() {
867+
return skiplist;
868+
}
869+
852870
@Override
853871
protected Explicit<Boolean> ignoreMalformed() {
854872
return ignoreMalformed;

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

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
8888
checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false));
8989
checker.registerConflictCheck("index", b -> b.field("index", false));
9090
checker.registerConflictCheck("store", b -> b.field("store", true));
91+
checker.registerConflictCheck("skip_list", b -> b.field("skip_list", true));
9192
checker.registerConflictCheck("format", b -> b.field("format", "yyyy-MM-dd"));
9293
checker.registerConflictCheck("print_format", b -> b.field("print_format", "yyyy-MM-dd"));
9394
checker.registerConflictCheck("locale", b -> b.field("locale", "es"));
@@ -643,4 +644,224 @@ public void testDateEncodePoint() {
643644
decoded = LongPoint.decodeDimension(encoded, 0);
644645
assertEquals("Should decrement epoch millis", negativeEpoch - 1, decoded);
645646
}
647+
648+
public void testSkipListParameterValidBooleanValues() throws IOException {
649+
// Test skip_list=true
650+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", true)));
651+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
652+
assertTrue("skip_list should be true", dateFieldMapper.skiplist());
653+
654+
// Test skip_list=false
655+
mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", false)));
656+
dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
657+
assertFalse("skip_list should be false", dateFieldMapper.skiplist());
658+
}
659+
660+
public void testSkipListParameterDefaultBehavior() throws IOException {
661+
// Test default behavior when skip_list parameter is omitted
662+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date")));
663+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
664+
assertFalse("skip_list should default to false", dateFieldMapper.skiplist());
665+
}
666+
667+
public void testSkipListParameterInvalidValues() throws IOException {
668+
// Test invalid string value
669+
MapperParsingException e = expectThrows(
670+
MapperParsingException.class,
671+
() -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", "invalid")))
672+
);
673+
assertThat(e.getMessage(), containsString("Failed to parse mapping"));
674+
675+
// Test invalid numeric value
676+
e = expectThrows(
677+
MapperParsingException.class,
678+
() -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", 123)))
679+
);
680+
assertThat(e.getMessage(), containsString("Failed to parse mapping"));
681+
682+
// Test null value
683+
e = expectThrows(
684+
MapperParsingException.class,
685+
() -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").nullField("skip_list")))
686+
);
687+
assertThat(e.getMessage(), containsString("Failed to parse mapping"));
688+
}
689+
690+
public void testSkipListParameterDateNanos() throws IOException {
691+
// Test skip_list=true with date_nanos type
692+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date_nanos").field("skip_list", true)));
693+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
694+
assertTrue("skip_list should be true for date_nanos", dateFieldMapper.skiplist());
695+
696+
// Test skip_list=false with date_nanos type
697+
mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date_nanos").field("skip_list", false)));
698+
dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
699+
assertFalse("skip_list should be false for date_nanos", dateFieldMapper.skiplist());
700+
701+
// Test default behavior for date_nanos
702+
mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date_nanos")));
703+
dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
704+
assertFalse("skip_list should default to false for date_nanos", dateFieldMapper.skiplist());
705+
}
706+
707+
// Integration tests for end-to-end skip_list functionality
708+
709+
public void testSkipListIntegrationDateFieldWithIndexedDocValues() throws IOException {
710+
// Test creating index with skip_list=true for date field
711+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", true)));
712+
713+
// Parse a document with date field
714+
ParsedDocument doc = mapper.parse(source(b -> b.field("field", "2016-03-11T10:30:00Z")));
715+
716+
// Verify the field structure
717+
IndexableField[] fields = doc.rootDoc().getFields("field");
718+
assertEquals("Should have 2 fields (point and doc values)", 2, fields.length);
719+
720+
// Verify point field
721+
IndexableField pointField = fields[0];
722+
assertEquals(1, pointField.fieldType().pointIndexDimensionCount());
723+
assertEquals(8, pointField.fieldType().pointNumBytes());
724+
assertFalse(pointField.fieldType().stored());
725+
assertEquals(1457692200000L, pointField.numericValue().longValue());
726+
727+
// Verify doc values field - when skip_list=true, should use indexed doc values
728+
IndexableField dvField = fields[1];
729+
assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType());
730+
assertEquals(1457692200000L, dvField.numericValue().longValue());
731+
assertFalse(dvField.fieldType().stored());
732+
733+
// Verify the mapper has skip_list enabled
734+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
735+
assertTrue("skip_list should be enabled", dateFieldMapper.skiplist());
736+
}
737+
738+
public void testSkipListIntegrationDateNanosFieldWithIndexedDocValues() throws IOException {
739+
// Test creating index with skip_list=true for date_nanos field
740+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date_nanos").field("skip_list", true)));
741+
742+
// Parse a document with date_nanos field
743+
ParsedDocument doc = mapper.parse(source(b -> b.field("field", "2016-03-11T10:30:00.123456789Z")));
744+
745+
// Verify the field structure
746+
IndexableField[] fields = doc.rootDoc().getFields("field");
747+
assertEquals("Should have 2 fields (point and doc values)", 2, fields.length);
748+
749+
// Verify point field
750+
IndexableField pointField = fields[0];
751+
assertEquals(1, pointField.fieldType().pointIndexDimensionCount());
752+
assertEquals(8, pointField.fieldType().pointNumBytes());
753+
assertFalse(pointField.fieldType().stored());
754+
755+
// Verify doc values field - when skip_list=true, should use indexed doc values
756+
IndexableField dvField = fields[1];
757+
assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType());
758+
assertFalse(dvField.fieldType().stored());
759+
760+
// Verify the mapper has skip_list enabled
761+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
762+
assertTrue("skip_list should be enabled", dateFieldMapper.skiplist());
763+
}
764+
765+
public void testSkipListIntegrationRegularDocValuesWhenDisabled() throws IOException {
766+
// Test creating index with skip_list=false for date field
767+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", false)));
768+
769+
// Parse a document with date field
770+
ParsedDocument doc = mapper.parse(source(b -> b.field("field", "2016-03-11T10:30:00Z")));
771+
772+
// Verify the field structure
773+
IndexableField[] fields = doc.rootDoc().getFields("field");
774+
assertEquals("Should have 2 fields (point and doc values)", 2, fields.length);
775+
776+
// Verify point field
777+
IndexableField pointField = fields[0];
778+
assertEquals(1, pointField.fieldType().pointIndexDimensionCount());
779+
assertEquals(8, pointField.fieldType().pointNumBytes());
780+
assertFalse(pointField.fieldType().stored());
781+
assertEquals(1457692200000L, pointField.numericValue().longValue());
782+
783+
// Verify doc values field - when skip_list=false, should use regular doc values
784+
IndexableField dvField = fields[1];
785+
assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType());
786+
assertEquals(1457692200000L, dvField.numericValue().longValue());
787+
assertFalse(dvField.fieldType().stored());
788+
789+
// Verify the mapper has skip_list disabled
790+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper.mappers().getMapper("field");
791+
assertFalse("skip_list should be disabled", dateFieldMapper.skiplist());
792+
}
793+
794+
public void testSkipListIntegrationWithMultipleResolutions() throws IOException {
795+
// Test both MILLISECONDS and NANOSECONDS resolution with skip_list enabled
796+
797+
// Test MILLISECONDS resolution (date type)
798+
DocumentMapper mapperMillis = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", true)));
799+
ParsedDocument docMillis = mapperMillis.parse(source(b -> b.field("field", "2016-03-11T10:30:00Z")));
800+
801+
IndexableField[] fieldsMillis = docMillis.rootDoc().getFields("field");
802+
assertEquals("Date field should have 2 fields", 2, fieldsMillis.length);
803+
assertEquals(1457692200000L, fieldsMillis[0].numericValue().longValue());
804+
assertEquals(1457692200000L, fieldsMillis[1].numericValue().longValue());
805+
806+
DateFieldMapper dateMapperMillis = (DateFieldMapper) mapperMillis.mappers().getMapper("field");
807+
assertTrue("Date field skip_list should be enabled", dateMapperMillis.skiplist());
808+
809+
// Test NANOSECONDS resolution (date_nanos type)
810+
DocumentMapper mapperNanos = createDocumentMapper(fieldMapping(b -> b.field("type", "date_nanos").field("skip_list", true)));
811+
ParsedDocument docNanos = mapperNanos.parse(source(b -> b.field("field", "2016-03-11T10:30:00.123456789Z")));
812+
813+
IndexableField[] fieldsNanos = docNanos.rootDoc().getFields("field");
814+
assertEquals("Date_nanos field should have 2 fields", 2, fieldsNanos.length);
815+
816+
DateFieldMapper dateMapperNanos = (DateFieldMapper) mapperNanos.mappers().getMapper("field");
817+
assertTrue("Date_nanos field skip_list should be enabled", dateMapperNanos.skiplist());
818+
}
819+
820+
public void testSkipListIntegrationMappingDefinitionSerialization() throws IOException {
821+
// Test that skip_list parameter appears correctly in mapping definitions
822+
823+
// Create mapper with skip_list=true
824+
MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "date").field("skip_list", true)));
825+
826+
// Get the field type and verify skip_list is set
827+
DateFieldMapper dateFieldMapper = (DateFieldMapper) mapperService.documentMapper().mappers().getMapper("field");
828+
assertTrue("skip_list should be true in mapper", dateFieldMapper.skiplist());
829+
830+
// Test with date_nanos as well
831+
MapperService mapperServiceNanos = createMapperService(fieldMapping(b -> b.field("type", "date_nanos").field("skip_list", true)));
832+
DateFieldMapper dateFieldMapperNanos = (DateFieldMapper) mapperServiceNanos.documentMapper().mappers().getMapper("field");
833+
assertTrue("skip_list should be true in date_nanos mapper", dateFieldMapperNanos.skiplist());
834+
}
835+
836+
public void testSkipListIntegrationFieldBehaviorConsistency() throws IOException {
837+
// Test that field behavior is consistent between skip_list enabled and disabled
838+
839+
String dateValue = "2016-03-11T10:30:00Z";
840+
long expectedTimestamp = 1457692200000L;
841+
842+
// Test with skip_list=true
843+
DocumentMapper mapperEnabled = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", true)));
844+
ParsedDocument docEnabled = mapperEnabled.parse(source(b -> b.field("field", dateValue)));
845+
846+
// Test with skip_list=false
847+
DocumentMapper mapperDisabled = createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("skip_list", false)));
848+
ParsedDocument docDisabled = mapperDisabled.parse(source(b -> b.field("field", dateValue)));
849+
850+
// Both should have the same field structure and values
851+
IndexableField[] fieldsEnabled = docEnabled.rootDoc().getFields("field");
852+
IndexableField[] fieldsDisabled = docDisabled.rootDoc().getFields("field");
853+
854+
assertEquals("Both should have same number of fields", fieldsEnabled.length, fieldsDisabled.length);
855+
assertEquals(
856+
"Point field values should match",
857+
fieldsEnabled[0].numericValue().longValue(),
858+
fieldsDisabled[0].numericValue().longValue()
859+
);
860+
assertEquals(
861+
"Doc values field values should match",
862+
fieldsEnabled[1].numericValue().longValue(),
863+
fieldsDisabled[1].numericValue().longValue()
864+
);
865+
assertEquals("Expected timestamp should match", expectedTimestamp, fieldsEnabled[0].numericValue().longValue());
866+
}
646867
}

0 commit comments

Comments
 (0)