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.

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

Mapping With a null Default Timestamp Causes NullPointerException on Merge #9205

Merged
merged 1 commit into from Jan 8, 2015

Conversation

dadoonet
Copy link
Member

@dadoonet dadoonet commented Jan 8, 2015

I have a field with a null default _timestamp value and when I try to update the mapping I get a server error caused by a NullPointerException

[2015-01-08 17:28:56,040][DEBUG][action.admin.indices.mapping.put] [...] failed to put mappings on indices [[feed_170_v1, feed_204_v1, feed_229_v1, feed_232_v1, feed_239_v1, feed_248_v1, feed_268_v1, feed_256_v1, feed_272_v1, feed_159_v1, feed_255_v1, feed_164_v1, feed_259_v1, feed_266_v1, feed_188_v1, feed_240_v1, feed_233_v1, feed_13_v1, feed_184_v1, feed_261_v1, feed_267_v1, feed_271_v1, feed_257_v1, feed_172_v1, feed_238_v1, feed_254_v1, feed_223_v1, feed_274_v1, feed_203_v1, feed_269_v1, feed_262_v1, feed_205_v1, feed_168_v1, feed_219_v1, feed_253_v1, feed_251_v1, feed_173_v1, feed_252_v1, feed_210_v1, feed_216_v1, feed_218_v1, feed_118_v1, feed_273_v1, feed_227_v1, feed_166_v1, feed_213_v1, feed_226_v1]], type [history]
java.lang.NullPointerException
        at org.elasticsearch.index.mapper.internal.TimestampFieldMapper.merge(TimestampFieldMapper.java:287)
        at org.elasticsearch.index.mapper.object.ObjectMapper.merge(ObjectMapper.java:936)
        at org.elasticsearch.index.mapper.DocumentMapper.merge(DocumentMapper.java:693)
        at org.elasticsearch.cluster.metadata.MetaDataMappingService$4.execute(MetaDataMappingService.java:508)
        at org.elasticsearch.cluster.service.InternalClusterService$UpdateTask.run(InternalClusterService.java:329)
        at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:153)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
        at java.lang.Thread.run(Thread.java:745)

https://github.com/elasticsearch/elasticsearch/blob/v1.4.2/src/main/java/org/elasticsearch/index/mapper/internal/TimestampFieldMapper.java#L286

Looks like the existence of default timestamp is not checked before use. The next line also has the same issue -- uses of default timestamp without checked to see if it's not null.

To reproduce:

$ curl -XPUT localhost:9200/twitter2
# {"acknowledged":true}

$ curl -XPUT localhost:9200/twitter2/tweet/_mapping -d '{
     "tweet" : {
         "_timestamp" : {
             "enabled" : true,
             "default" : null
         }
     }
}'
# {"acknowledged":true}

$ curl -XPUT localhost:9200/twitter2/tweet/_mapping -d '{
     "tweet" : {
         "_timestamp" : {
             "enabled" : true,
             "default" : null
         },
         "properties": {
             "user": {"type": "string"}
         }
     }
}'
# {"error":"NullPointerException[null]","status":500} Yikes!

Closes #9204.

@dadoonet dadoonet added the review label Jan 8, 2015
@dadoonet dadoonet self-assigned this Jan 8, 2015
if (timestampFieldMapperMergeWith.defaultTimestamp() == null && defaultTimestamp == null) {
return;
}
if (timestampFieldMapperMergeWith.defaultTimestamp() != null && defaultTimestamp == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just do the == null part of this and the next if condition, since if the other were equal to null, we would have already returned in the first if?

if (x == null && y == null) return
if (x == null) ...
else if (y == null) ...
else // x and y are not null

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very true!

@rjernst
Copy link
Member

rjernst commented Jan 8, 2015

LGTM, just 2 minor comments.

@dadoonet
Copy link
Member Author

dadoonet commented Jan 8, 2015

Thanks. PR updated.

@rjernst
Copy link
Member

rjernst commented Jan 8, 2015

LGTM

…n Merge

I have a field with a `null` [default `_timestamp` value](http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/mapping-timestamp-field.html#mapping-timestamp-field-default) and when I try to update the mapping I get a server error caused by a `NullPointerException`

```
[2015-01-08 17:28:56,040][DEBUG][action.admin.indices.mapping.put] [...] failed to put mappings on indices [[feed_170_v1, feed_204_v1, feed_229_v1, feed_232_v1, feed_239_v1, feed_248_v1, feed_268_v1, feed_256_v1, feed_272_v1, feed_159_v1, feed_255_v1, feed_164_v1, feed_259_v1, feed_266_v1, feed_188_v1, feed_240_v1, feed_233_v1, feed_13_v1, feed_184_v1, feed_261_v1, feed_267_v1, feed_271_v1, feed_257_v1, feed_172_v1, feed_238_v1, feed_254_v1, feed_223_v1, feed_274_v1, feed_203_v1, feed_269_v1, feed_262_v1, feed_205_v1, feed_168_v1, feed_219_v1, feed_253_v1, feed_251_v1, feed_173_v1, feed_252_v1, feed_210_v1, feed_216_v1, feed_218_v1, feed_118_v1, feed_273_v1, feed_227_v1, feed_166_v1, feed_213_v1, feed_226_v1]], type [history]
java.lang.NullPointerException
        at org.elasticsearch.index.mapper.internal.TimestampFieldMapper.merge(TimestampFieldMapper.java:287)
        at org.elasticsearch.index.mapper.object.ObjectMapper.merge(ObjectMapper.java:936)
        at org.elasticsearch.index.mapper.DocumentMapper.merge(DocumentMapper.java:693)
        at org.elasticsearch.cluster.metadata.MetaDataMappingService$4.execute(MetaDataMappingService.java:508)
        at org.elasticsearch.cluster.service.InternalClusterService$UpdateTask.run(InternalClusterService.java:329)
        at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:153)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
        at java.lang.Thread.run(Thread.java:745)
```

https://github.com/elasticsearch/elasticsearch/blob/v1.4.2/src/main/java/org/elasticsearch/index/mapper/internal/TimestampFieldMapper.java#L286

Looks like the existence of default timestamp is not checked before use. The next line also has the same issue -- uses of default timestamp without checked to see if it's not null.

To reproduce:

```
$ curl -XPUT localhost:9200/twitter2

$ curl -XPUT localhost:9200/twitter2/tweet/_mapping -d '{
     "tweet" : {
         "_timestamp" : {
             "enabled" : true,
             "default" : null
         }
     }
}'

$ curl -XPUT localhost:9200/twitter2/tweet/_mapping -d '{
     "tweet" : {
         "_timestamp" : {
             "enabled" : true,
             "default" : null
         },
         "properties": {
             "user": {"type": "string"}
         }
     }
}'
```

Closes elastic#9204.
@dadoonet dadoonet removed the review label Jan 8, 2015
@dadoonet dadoonet merged commit 62c6d63 into elastic:1.4 Jan 8, 2015
@dadoonet dadoonet deleted the pr/9204-timestamp-npe branch January 8, 2015 20:34
@clintongormley clintongormley added >regression :Search/Mapping Index mappings, including merging and defining field types v1.4.3 labels Jan 28, 2015
@clintongormley clintongormley changed the title Mapping With a null Default Timestamp Causes NullPointerException on Merge Mapping: Mapping With a null Default Timestamp Causes NullPointerException on Merge Feb 10, 2015
@clintongormley clintongormley changed the title Mapping: Mapping With a null Default Timestamp Causes NullPointerException on Merge Mapping With a null Default Timestamp Causes NullPointerException on Merge Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>regression :Search/Mapping Index mappings, including merging and defining field types v1.4.3 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants