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

Timestamp index settings incorrectly stored #3174

Closed
brusic opened this issue Jun 12, 2013 · 5 comments
Closed

Timestamp index settings incorrectly stored #3174

brusic opened this issue Jun 12, 2013 · 5 comments

Comments

@brusic
Copy link
Contributor

brusic commented Jun 12, 2013

When setting a timestamp field as not indexed, the value is not saved correctly in the cluster state.

Using the following index template:

{
    "foo_template": {
        "template": "foo-*",
        "settings": {
            "index.number_of_shards": 1,
            "index.number_of_replicas": 0
        },
        "mappings": {
            "type1": {
                "_timestamp" : { "enabled" : true, "index": "no", "store": "yes"},
                "properties": {
                    "test": {"type" : "long", "index" : "no"},
                    "baz": {"type" : "boolean", "index" : "no"}
                }
            }
        }
    }
}

Creating a new index (foo-test) with that template works correctly. The mapping returned is as follows:

{
  "foo-test": {
    "type1": {
      "_timestamp": {
        "enabled": true,
        "index": false,
        "store": true
      },
      "properties": {
        "baz": {
          "type": "boolean",
          "index": "no"
        },
        "test": {
          "type": "long",
          "index": "no"
        }
      }
    }
  }
}

Notice that the values for the timestamp field now uses boolean values instead of yes/no.
Whenever the cluster state is recovered, the boolean values are still used, causing an exception

[2013-06-12 16:34:43,481][INFO ][gateway                  ] [searchnode] recovered [1] indices into cluster_state
[2013-06-12 16:34:43,482][DEBUG][indices.cluster          ] [searchnode] [foo-test] adding mapping [type1], source [{"type1":{"_timestamp":{"enabled":true,"index":false,"store":true},"properties":{"baz":{"type":"boolean","index":"no"},"test":{"type":"long","index":"no"}}}}]
[2013-06-12 16:34:43,482][WARN ][indices.cluster          ] [searchnode] [foo-test] failed to add mapping [type1], source [{"type1":{"_timestamp":{"enabled":true,"index":false,"store":true},"properties":{"baz":{"type":"boolean","index":"no"},"test":{"type":"long","index":"no"}}}}]
org.elasticsearch.index.mapper.MapperParsingException: Wrong value for index [false] for field [_timestamp]

The index setting on the timestamp field works correctly on 0.20.0RC1. The biggest change is the toXContent method. The build sets the fields as a boolean: builder.field("index", fieldType.indexed()), which works on other field mappers.

I have not ran the test in a debugger yet, but will do so shortly.

The timestamp field in 0.20.0RC1

_timestamp: {
    enabled: true
    index: no
    store: yes
}
@brusic
Copy link
Contributor Author

brusic commented Jun 13, 2013

The fix should be setting the field as:
builder.field("index", indexTokenizeOptionToString(fieldType.indexed(), fieldType.tokenized()));

Can submit a pull request, but it's a one line change. :) Will do so anyways later on. This issue probably affects the RoutingFieldMapper as well.

@s1monw
Copy link
Contributor

s1monw commented Jun 13, 2013

a PR would be awesome maybe including a testcase?

@brusic
Copy link
Contributor Author

brusic commented Jun 13, 2013

This issue is one of those times when the test case will be much longer than the fix.:)

I will attempt do have a PR shortly. This feature is not essential in my system and I am in the middle of a Lucene 4.3/elasticsearch 0.90 upgrade. However, if 0.90.2 will be released soon, I will work on it first. Any word on its release?

@brusic
Copy link
Contributor Author

brusic commented Jul 5, 2013

I finally had time to fix this issue today (day after a holiday is slow around here), and you already fixed it. The fix is easy, but will the current tests simulate the serialization/deserialization of the index settings? Most tests skip this step.

@brusic
Copy link
Contributor Author

brusic commented Jul 5, 2013

The routing field also needs to be fixed:
brusic@a660fcc

(I accidentally created a git branch off another branch, if not I would submit a pull request)

spinscale added a commit to spinscale/elasticsearch that referenced this issue Jul 15, 2013
The index field was serialized as a boolean instead of showing the
'analyed', 'not_analzyed', 'no' options. Fixed by calling
indexTokenizeOptionToString() in the builder.

Closes elastic#3174
spinscale added a commit that referenced this issue Jul 15, 2013
The index field was serialized as a boolean instead of showing the
'analyed', 'not_analzyed', 'no' options. Fixed by calling
indexTokenizeOptionToString() in the builder.

Closes #3174
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
The index field was serialized as a boolean instead of showing the
'analyed', 'not_analzyed', 'no' options. Fixed by calling
indexTokenizeOptionToString() in the builder.

Closes elastic#3174
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants