Restrict fields with the same name in different types to have the same core settings

[rjernst]

We currently are very lax about allowing data types to conflict for the
same field name, across document types. This change makes the underlying
map in MapperService a 1-1 map of field name to field type, and throws
exception when new types are not compatible.

To still allow changing a type, with parameters that are allowed to be
changed, but for a field that exists in multiple types, a new parameter
to index creation and put mapping API is added: update_all_types.
This defaults to false, and the exception messages suggest using
this parameter when trying to modify a setting that is allowed to be
modified but is being limited by this restriction.

There are also a couple changes which try to base fields from new types
for dynamic mappings, and root mappers, on existing settings. For
dynamic mappings this is important if the dynamic defaults have been
changed. For root mappings, this is mostly just for backcompat when
pre 2.0 root mappers could have their field type changed.

fixes #8871

[jpountz]

s/give/given/

[jpountz]

The change looks great. I also played with the change, trying to add conflicting mappings at index creation time, through dynamic mappings and explicit mappings updates and things seem to work fine overall. I found some bugs wrt dynamic mappings that I fixed in your branch (hope you don't mind) and left comments for some other bugs.

[jpountz]

When I did the following test:

DELETE test

PUT test 
{
  "mappings": {
    "test1": {
      "properties": {
        "s": {
          "type": "binary"
        }
      }
    }
  }
}

PUT test/test2/_mapping
{
  "test2": {
    "properties": {
      "s": {
        "type": "string"
      }
    }
  }
}

I got the following response:

{
   "error": {
      "root_cause": [
         {
            "type": "illegal_argument_exception",
            "reason": "Mapper for [s] conflicts with existing mapping in other types[mapper [s] has different index values]"
         }
      ],
      "type": "illegal_argument_exception",
      "reason": "Mapper for [s] conflicts with existing mapping in other types[mapper [s] has different index values]"
   },
   "status": 400
}

I was surprised to see an error about options while the mapped type isn't even the same. I suspect it comes from this method which starts validating options without first comparing the type implementation?

[rjernst]

I agree we need a better error message here. We can't just check the class like mappers do, since some mappers use MappedFieldType directly. I think I will add a setting to MappedFieldType which is the "typename"? This will give a much better error than what we get now IMO (the actual type, instead of a classname the user knows nothing about).

[s1monw]

I left a bunch of comments - I'd like to talk the concurrency aspects through with you in person here and I also wonder if we can get rid of MapperAnalyzer.java now?

[rjernst]

@s1monw @jpountz I pushed a new commit. The one outstanding thing is the better error message for conflicting data types.

As for MapperAnalyzer.java, I would love to get rid of this, but afaict right now it is still the one thing that restricts a document to have access only to the fields defined within a type?

[rjernst]

Actually, now I'm not sure about MapperAnalyzer.java. The DocumentParser looks up fields there when parsing, so by the time the analyzer needs to lookup the field, we should be able to just lookup the fieldtype for the index. I will try removing it.

[rjernst]

@jpountz @s1monw I removed MapperAnalyzer and added a better message when field types aren't the same.

[jpountz]

LGTM!

[s1monw]

I removed MapperAnalyzer and added a better message when field types aren't the same.

awesome!!!! - please open a followup for the craziness around concurrent maps etc.

LGTM