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

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

Closed

Conversation

dadoonet
Copy link
Member

PR #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 #8882.

… 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.
@dadoonet dadoonet added review :Search/Mapping Index mappings, including merging and defining field types labels Dec 23, 2014
@rjernst
Copy link
Member

rjernst commented Dec 23, 2014

The "default default" should not change dynamically based on how other settings are set, IMO. That would be very confusing for users. Whether the timestamp comes in _timestamp or a custom set field through path, if we want the default to be now, then it should always be so.

@dadoonet
Copy link
Member Author

@rjernst In that case, how to force a rejection when timestamp is not provided within the json document (when using path)?
Does this mean we should introduce a new setting like reject_null: true or something?
Or using another special value instead of now like "default":"mandatory"?

@rjernst
Copy link
Member

rjernst commented Dec 23, 2014

How do we require other fields? I thought it was with default: null? I think null should always be rejected?

@dadoonet
Copy link
Member Author

The problem is that some languages (.Net) can not explicitly set default: null. See #8882.

Let's say we use now as a default value. If a user wants to define a mapping with a timestamp provided by path and wants to reject any document which does not provide a value, he needs to define in mapping default: null. Which seems to be not possible with .Net client.

@rjernst
Copy link
Member

rjernst commented Dec 23, 2014

The problem is that some languages (.Net) can not explicitly set default: null

Then isn't this a problem for all field types, not just timestamp?

@dadoonet
Copy link
Member Author

I think it's the only field for which null has a special meaning. Right @Mpdreamz?

@rjernst
Copy link
Member

rjernst commented Dec 23, 2014

If there are problems with using null in .Net, then there should be a separate setting for requiring the field. But it should be validated that only one is used.

@dadoonet
Copy link
Member Author

Like ignore_missing suggested in original issue?

@rjernst
Copy link
Member

rjernst commented Dec 29, 2014

Sure, sounds good to me. :)

@dadoonet
Copy link
Member Author

Closing in favor of #9104.

@dadoonet dadoonet closed this Dec 30, 2014
@dadoonet dadoonet deleted the issue/8882-timestamp-default branch January 17, 2015 13:13
dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Jan 20, 2015
Related to elastic#9049.

By default, the default value for `timestamp` is `now` which means the date the document was processed by the indexing chain.

You can now reject documents which not provide a `timestamp` value by setting `ignore_missing` to false (default to `true`):

```js
{
    "tweet" : {
        "_timestamp" : {
            "enabled" : true,
            "ignore_missing" : false
        }
    }
}
```

When you update the cluster to 1.5 or master, this index created with 1.4 we automatically migrate an index created with 1.4 to the 1.5 syntax.

Let say you have defined this in elasticsearch 1.4.x:

```js
DELETE test
PUT test
{
  "settings": {
    "number_of_shards": 1,
    "number_of_replicas": 0
  }
}
PUT test/type/_mapping
{
  "type" : {
      "_timestamp" : {
          "enabled" : true,
          "default" : null
      }
  }
}
```

After migration, the mapping become:

```js
{
   "test": {
      "mappings": {
         "type": {
            "_timestamp": {
               "enabled": true,
               "store": false,
               "ignore_missing": false
            },
            "properties": {}
         }
      }
   }
}
```

Closes elastic#8882.
dadoonet added a commit that referenced this pull request Jan 20, 2015
Related to #9049.

By default, the default value for `timestamp` is `now` which means the date the document was processed by the indexing chain.

You can now reject documents which not provide a `timestamp` value by setting `ignore_missing` to false (default to `true`):

```js
{
    "tweet" : {
        "_timestamp" : {
            "enabled" : true,
            "ignore_missing" : false
        }
    }
}
```

When you update the cluster to 1.5 or master, this index created with 1.4 we automatically migrate an index created with 1.4 to the 1.5 syntax.

Let say you have defined this in elasticsearch 1.4.x:

```js
DELETE test
PUT test
{
  "settings": {
    "number_of_shards": 1,
    "number_of_replicas": 0
  }
}
PUT test/type/_mapping
{
  "type" : {
      "_timestamp" : {
          "enabled" : true,
          "default" : null
      }
  }
}
```

After migration, the mapping become:

```js
{
   "test": {
      "mappings": {
         "type": {
            "_timestamp": {
               "enabled": true,
               "store": false,
               "ignore_missing": false
            },
            "properties": {}
         }
      }
   }
}
```

Closes #8882.
(cherry picked from commit fb10346)
dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Jan 26, 2015
When creating an index with:

```
PUT new_index
{
    "mappings": {
        "power": {
            "_timestamp" : {
                "enabled" : true,
                "default": null
            }
        }
    }
}
```

When restarting the cluster, `now` is applied instead of `null`. So index become:

```
{
    "mappings": {
        "power": {
            "_timestamp" : {
                "enabled" : true,
                "default": "now"
            }
        }
    }
}
```

This PR fix that and applies `null` when it was explicitly set.

Note that this won't happen anymore in 1.5 as `null` is not allowed anymore as a `default` value. See elastic#9104.

See also:

* elastic#7036
* elastic#9049
* elastic#9426#issuecomment-71534871
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
When creating an index with:

```
PUT new_index
{
    "mappings": {
        "power": {
            "_timestamp" : {
                "enabled" : true,
                "default": null
            }
        }
    }
}
```

When restarting the cluster, `now` is applied instead of `null`. So index become:

```
{
    "mappings": {
        "power": {
            "_timestamp" : {
                "enabled" : true,
                "default": "now"
            }
        }
    }
}
```

This PR fix that and applies `null` when it was explicitly set.

Note that this won't happen anymore in 1.5 as `null` is not allowed anymore as a `default` value. See elastic#9104.

See also:

* elastic#7036
* elastic#9049
* elastic#9426#issuecomment-71534871
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search/Mapping Index mappings, including merging and defining field types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New timestamp default option changed the default behavior for missing paths
3 participants