[5.0 Breaking] Incorrect numeric NumberType mappings are created.

[niemyjski]

In 1.x client the mappings were (mostly, with a bigger datatype) correctly created, now with the 5.x mappings everything I've come across seems to be mapped to a float.. It seems we are not setting the correct NumberType.

.Number(f => f.Name(e => e.Value).Alias(Alias.Value)) // value is of type decimal?
.Number(f => f.Name(e => e.Count).Alias(Alias.Count)) // count is of type int?

previous versions of the mapping would have created these as (mostly) correct types:

{
               "value": {
                  "type": "double",
                  "include_in_all": false
               },
               "count": {
                  "type": "double",
                  "include_in_all": false
               },
}

However, they are currently being submitted as

{
          "value": {
            "type": "float"
          },
          "count": {
            "type": "float"
          }
}

Looking at https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html the types should be

{
          "value": {
            "type": "double"
          },
          "count": {
            "type": "integer"
          }
}

The value here, I wonder should be double or maybe a scaled_float ?

[russcam]

The default mapping for a floating point number changed from being mapped as a double to float in Elasticsearch 5.0 - search for #15319. (see elastic/elasticsearch#15319 for the issue).

With the numeric fluent mapping above, because the .Type(NumberType type) is not specified, they default to float to align with the Elasticsearch default. include_in_all is also false by default so is not sent.

[niemyjski]

I don't agree with this behavior because NEST has all the type information by .NET why should we be creating bytes as floats etc... The elastic lib should handle this and not make us think about this. Can we get this changed so we get the smallest datatype possible based on our models?

[niemyjski]

It would make sense to always send float if we didn't have the type info but we know it's a numeric type and we have strongly typed information saying what the data type is and we all know the numeric ranges and what they map to. Thus we can make our elastic search cluster even faster out of the box while reducing constraints due to float defaults for every single numeric field.

[Mpdreamz]

.AutoMap() does the right thing here.

When it gets to Number(n=>n.Name(p=>p.Prop)) things get a lot hairier though

Number(n=>n.Name(Field<PropertyX>(p=>p.Prop)) is a lot more difficult to deconstruct but if we'd do it for the previous we'd have to do it here too.

then we'd need to handle Number(n=>n.Type().Name(p=>p.Prop)) and make sure it overrides whatever .Name() inferrs.

Not even mentioning how to deal with the object initializer syntax.

new TypeMapping
{
    Properties = new Properties
    {
        { Field<Project>(p=>p.SomeInt)), new NumberProperty(NumberType.Float) { LocalMetadata = new Dictionary<string, object> { { "foo", "bar" } } } }
    }
};

This is exactly the kind of merge binds we put ourselves in with < 2.x releases that we are looking to avoid.

I am not opposed to introducing more explicit .Float/Integer/ .Byte aliases though.

[niemyjski]

We should be able to get the type from the expression pretty easily. This could be used when the NumberType is null in the mappings. I think something has to be done as we are going to be generating bad mappings (and unless the end user is double checking these, they won't know) and people are going to run with it. I'm fine with adding aliases as long as they only work against the respected data types. It still would be nice to just keep it clean and infer the lowest NumberType possible