Skip to content

Commit a4488ed

Browse files
dadoonetdadoonet
committed
New timestamp default option changed the default behavior for missing paths
PR elastic#7036 changed the behavior for timestamp when provided as a parameter or within the document using `path` attribute. This PR now considers that: * when using timestamp as a parameter, we use a default value of `now`. Which means that if no timestamp is provided, the current time is used when the index operation is performed. * when getting the timestamp from `path`, we use a default value of `null`. Which means that if no value is provided within the document, indexation will fail. If users want to set the default value for `timestamp`, they can explicitly set one or set `"default": "now"`. Closes elastic#8882.
1 parent 6d87284 commit a4488ed

File tree

3 files changed

+74
-15
lines changed

3 files changed

+74
-15
lines changed

‎docs/reference/mapping/fields/timestamp-field.asciidoc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,24 @@ You can define a default value for when timestamp is not provided
9090
within the index request or in the `_source` document.
9191

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

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

9698
[source,js]
9799
--------------------------------------------------
98100
{
99101
"tweet" : {
100102
"_timestamp" : {
101103
"enabled" : true,
102-
"default" : null
104+
"path" : "post_date",
105+
"default" : "now"
103106
}
104107
}
105108
}
106109
--------------------------------------------------
107110

108-
If you don't provide any timestamp value, indexation will fail.
109-
110111
You can also set the default value to any date respecting <<mapping-timestamp-field-format,timestamp format>>:
111112

112113
[source,js]

‎src/main/java/org/elasticsearch/index/mapper/internal/TimestampFieldMapper.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ public static class Defaults extends DateFieldMapper.Defaults {
7474
public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_DISABLED;
7575
public static final String PATH = null;
7676
public static final FormatDateTimeFormatter DATE_TIME_FORMATTER = Joda.forPattern(DEFAULT_DATE_TIME_FORMAT);
77-
public static final String DEFAULT_TIMESTAMP = "now";
77+
public static final String NOW_TIMESTAMP = "now";
78+
public static final String DEFAULT_TIMESTAMP = NOW_TIMESTAMP;
7879
}
7980

8081
public static class Builder extends NumberFieldMapper.Builder<Builder, TimestampFieldMapper> {
@@ -129,6 +130,8 @@ public TimestampFieldMapper build(BuilderContext context) {
129130
public static class TypeParser implements Mapper.TypeParser {
130131
@Override
131132
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
133+
boolean pathSet = false;
134+
boolean defaultSet = false;
132135
TimestampFieldMapper.Builder builder = timestamp();
133136
parseField(builder, builder.name, node, parserContext);
134137
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
@@ -141,15 +144,23 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
141144
iterator.remove();
142145
} else if (fieldName.equals("path")) {
143146
builder.path(fieldNode.toString());
147+
pathSet = true;
144148
iterator.remove();
145149
} else if (fieldName.equals("format")) {
146150
builder.dateTimeFormatter(parseDateTimeFormatter(fieldNode.toString()));
147151
iterator.remove();
148152
} else if (fieldName.equals("default")) {
149153
builder.defaultTimestamp(fieldNode == null ? null : fieldNode.toString());
154+
defaultSet = true;
150155
iterator.remove();
151156
}
152157
}
158+
159+
// When path is used and no default value is not explicitly set, we force it to null.
160+
if (pathSet && defaultSet == false) {
161+
builder.defaultTimestamp(null);
162+
}
163+
153164
return builder;
154165
}
155166
}
@@ -283,7 +294,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
283294
if (includeDefaults || !dateTimeFormatter.format().equals(Defaults.DATE_TIME_FORMATTER.format())) {
284295
builder.field("format", dateTimeFormatter.format());
285296
}
286-
if (includeDefaults || !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) {
297+
// We print `default` if explicitly required or if not using path with default != now
298+
// or if using using path with default != null
299+
if (includeDefaults || (path == Defaults.PATH && !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) ||
300+
(path != Defaults.PATH && defaultTimestamp != null)) {
287301
builder.field("default", defaultTimestamp);
288302
}
289303
if (customFieldDataSettings != null) {
@@ -305,7 +319,7 @@ public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappi
305319
this.enabledState = timestampFieldMapperMergeWith.enabledState;
306320
}
307321
} else {
308-
if (!timestampFieldMapperMergeWith.defaultTimestamp().equals(defaultTimestamp)) {
322+
if (timestampFieldMapperMergeWith.defaultTimestamp() != null && !timestampFieldMapperMergeWith.defaultTimestamp().equals(defaultTimestamp)) {
309323
mergeContext.addConflict("Cannot update default in _timestamp value. Value is " + defaultTimestamp.toString() + " now encountering " + timestampFieldMapperMergeWith.defaultTimestamp());
310324
}
311325
if (this.path != null) {

‎src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public void testThatSerializationWorksCorrectlyForIndexField() throws Exception
168168
assertThat(timestampConfiguration.get("index").toString(), is("no"));
169169
}
170170

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

189189
IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
190190
request.process(metaData, mappingMetaData, true, "test");
191-
assertThat(request.timestamp(), notNullValue());
192-
193-
// We should have less than one minute (probably some ms)
194-
long delay = System.currentTimeMillis() - Long.parseLong(request.timestamp());
195-
assertThat(delay, lessThanOrEqualTo(60000L));
196191
}
197192

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

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

355350
@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
356-
public void testTimestampMissingShouldFail() throws Exception {
351+
public void testPathMissingShouldFail() throws Exception {
352+
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
353+
.startObject("_timestamp")
354+
.field("enabled", "yes")
355+
.field("path", "timestamp")
356+
.endObject()
357+
.endObject().endObject();
358+
XContentBuilder doc = XContentFactory.jsonBuilder()
359+
.startObject()
360+
.field("foo", "bar")
361+
.endObject();
362+
363+
MetaData metaData = MetaData.builder().build();
364+
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping.string());
365+
366+
MappingMetaData mappingMetaData = new MappingMetaData(docMapper);
367+
368+
IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
369+
request.process(metaData, mappingMetaData, true, "test");
370+
}
371+
372+
@Test(expected = TimestampParsingException.class) // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
373+
public void testTimestampMissingWithForcedNullDefaultShouldFail() throws Exception {
357374
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
358375
.startObject("_timestamp")
359376
.field("enabled", "yes")
@@ -374,6 +391,33 @@ public void testTimestampMissingShouldFail() throws Exception {
374391
request.process(metaData, mappingMetaData, true, "test");
375392
}
376393

394+
@Test // Issue 4718: was throwing a TimestampParsingException: failed to parse timestamp [null]
395+
public void testTimestampMissingShouldNotFail() throws Exception {
396+
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
397+
.startObject("_timestamp")
398+
.field("enabled", "yes")
399+
.endObject()
400+
.endObject().endObject();
401+
XContentBuilder doc = XContentFactory.jsonBuilder()
402+
.startObject()
403+
.field("foo", "bar")
404+
.endObject();
405+
406+
MetaData metaData = MetaData.builder().build();
407+
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping.string());
408+
409+
MappingMetaData mappingMetaData = new MappingMetaData(docMapper);
410+
411+
IndexRequest request = new IndexRequest("test", "type", "1").source(doc);
412+
request.process(metaData, mappingMetaData, true, "test");
413+
414+
assertThat(request.timestamp(), notNullValue());
415+
416+
// We should have less than one minute (probably some ms)
417+
long delay = System.currentTimeMillis() - Long.parseLong(request.timestamp());
418+
assertThat(delay, lessThanOrEqualTo(60000L));
419+
}
420+
377421
@Test
378422
public void testDefaultTimestampStream() throws IOException {
379423
// Testing null value for default timestamp

0 commit comments

Comments
 (0)