Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New timestamp default option changed the default behavior for missing paths #9049

Closed
wants to merge 1 commit into from
Closed

New timestamp default option changed the default behavior for missing paths #9049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions docs/reference/mapping/fields/timestamp-field.asciidoc
View file
Open in desktop
Expand Up @@ -90,23 +90,24 @@ You can define a default value for when timestamp is not provided
within the index request or in the `_source` document.

By default, the default value is `now` which means the date the document was processed by the indexing chain.
But if you are using `path`, there is no default value which means that indexing will fail if your document does
not provide the date.

You can disable that default value by setting `default` to `null`. It means that `timestamp` is mandatory:
You force also the default value for path by setting `default` to `now`:

[source,js]
--------------------------------------------------
{
"tweet" : {
"_timestamp" : {
"enabled" : true,
"default" : null
"path" : "post_date",
"default" : "now"
}
}
}
--------------------------------------------------

If you don't provide any timestamp value, indexation will fail.

You can also set the default value to any date respecting <<mapping-timestamp-field-format,timestamp format>>:

[source,js]
Expand Down
20 changes: 17 additions & 3 deletions src/main/java/org/elasticsearch/index/mapper/internal/TimestampFieldMapper.java
View file
Open in desktop
Expand Up @@ -74,7 +74,8 @@ public static class Defaults extends DateFieldMapper.Defaults {
public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_DISABLED;
public static final String PATH = null;
public static final FormatDateTimeFormatter DATE_TIME_FORMATTER = Joda.forPattern(DEFAULT_DATE_TIME_FORMAT);
public static final String DEFAULT_TIMESTAMP = "now";
public static final String NOW_TIMESTAMP = "now";
public static final String DEFAULT_TIMESTAMP = NOW_TIMESTAMP;
}

public static class Builder extends NumberFieldMapper.Builder<Builder, TimestampFieldMapper> {
Expand Down Expand Up @@ -129,6 +130,8 @@ public TimestampFieldMapper build(BuilderContext context) {
public static class TypeParser implements Mapper.TypeParser {
@Override
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
boolean pathSet = false;
boolean defaultSet = false;
TimestampFieldMapper.Builder builder = timestamp();
parseField(builder, builder.name, node, parserContext);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Expand All @@ -141,15 +144,23 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
iterator.remove();
} else if (fieldName.equals("path")) {
builder.path(fieldNode.toString());
pathSet = true;
iterator.remove();
} else if (fieldName.equals("format")) {
builder.dateTimeFormatter(parseDateTimeFormatter(fieldNode.toString()));
iterator.remove();
} else if (fieldName.equals("default")) {
builder.defaultTimestamp(fieldNode == null ? null : fieldNode.toString());
defaultSet = true;
iterator.remove();
}
}

// When path is used and no default value is not explicitly set, we force it to null.
if (pathSet && defaultSet == false) {
builder.defaultTimestamp(null);
}

return builder;
}
}
Expand Down Expand Up @@ -283,7 +294,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (includeDefaults || !dateTimeFormatter.format().equals(Defaults.DATE_TIME_FORMATTER.format())) {
builder.field("format", dateTimeFormatter.format());
}
if (includeDefaults || !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) {
// We print `default` if explicitly required or if not using path with default != now
// or if using using path with default != null
if (includeDefaults || (path == Defaults.PATH && !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) ||
(path != Defaults.PATH && defaultTimestamp != null)) {
builder.field("default", defaultTimestamp);
}
if (customFieldDataSettings != null) {
Expand All @@ -305,7 +319,7 @@ public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappi
this.enabledState = timestampFieldMapperMergeWith.enabledState;
}
} else {
if (!timestampFieldMapperMergeWith.defaultTimestamp().equals(defaultTimestamp)) {
if (timestampFieldMapperMergeWith.defaultTimestamp() != null && !timestampFieldMapperMergeWith.defaultTimestamp().equals(defaultTimestamp)) {
mergeContext.addConflict("Cannot update default in _timestamp value. Value is " + defaultTimestamp.toString() + " now encountering " + timestampFieldMapperMergeWith.defaultTimestamp());
}
if (this.path != null) {
Expand Down
60 changes: 52 additions & 8 deletions src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java
View file
Open in desktop
Expand Up @@ -168,7 +168,7 @@ public void testThatSerializationWorksCorrectlyForIndexField() throws Exception
assertThat(timestampConfiguration.get("index").toString(), is("no"));
}

@Test // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
public void testPathMissingDefaultValue() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_timestamp")
Expand All @@ -188,11 +188,6 @@ public void testPathMissingDefaultValue() throws Exception {

IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
request.process(metaData, mappingMetaData, true, "test");
assertThat(request.timestamp(), notNullValue());

// We should have less than one minute (probably some ms)
long delay = System.currentTimeMillis() - Long.parseLong(request.timestamp());
assertThat(delay, lessThanOrEqualTo(60000L));
}

@Test // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
Expand Down Expand Up @@ -330,7 +325,7 @@ public void testTimestampMissingNowDefaultValue() throws Exception {
}

@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
public void testPathMissingShouldFail() throws Exception {
public void testPathMissingWithForcedNullDefaultShouldFail() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_timestamp")
.field("enabled", "yes")
Expand All @@ -353,7 +348,29 @@ public void testPathMissingShouldFail() throws Exception {
}

@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
public void testTimestampMissingShouldFail() throws Exception {
public void testPathMissingShouldFail() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_timestamp")
.field("enabled", "yes")
.field("path", "timestamp")
.endObject()
.endObject().endObject();
XContentBuilder doc = XContentFactory.jsonBuilder()
.startObject()
.field("foo", "bar")
.endObject();

MetaData metaData = MetaData.builder().build();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping.string());

MappingMetaData mappingMetaData = new MappingMetaData(docMapper);

IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
request.process(metaData, mappingMetaData, true, "test");
}

@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
public void testTimestampMissingWithForcedNullDefaultShouldFail() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_timestamp")
.field("enabled", "yes")
Expand All @@ -374,6 +391,33 @@ public void testTimestampMissingShouldFail() throws Exception {
request.process(metaData, mappingMetaData, true, "test");
}

@Test // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
public void testTimestampMissingShouldNotFail() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_timestamp")
.field("enabled", "yes")
.endObject()
.endObject().endObject();
XContentBuilder doc = XContentFactory.jsonBuilder()
.startObject()
.field("foo", "bar")
.endObject();

MetaData metaData = MetaData.builder().build();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping.string());

MappingMetaData mappingMetaData = new MappingMetaData(docMapper);

IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
request.process(metaData, mappingMetaData, true, "test");

assertThat(request.timestamp(), notNullValue());

// We should have less than one minute (probably some ms)
long delay = System.currentTimeMillis() - Long.parseLong(request.timestamp());
assertThat(delay, lessThanOrEqualTo(60000L));
}

@Test
public void testDefaultTimestampStream() throws IOException {
// Testing null value for default timestamp
Expand Down