Teach _search to read search time mappings #59316

nik9000 · 2020-07-09T16:13:24Z

This adds support for search time mappings to the _search API like so:

curl -XPOST -HContent-Type:application/json -uelastic:password 'localhost:9200/test/_search?pretty&error_trace' -d'{
  "runtime_mappings": {
    "cat": {
      "type": "script",
      "runtime_type": "keyword",
      "script": "value(\"cat\")"
    }
  },
  "docvalue_fields": ["cat"]
}'

It just lifts the mappings field from the create index and mappings
update requests and drops it into the _search request. It then gives
each mapped field a chance to convert itself into a runtime field. Most
mapping can not and will throw an exception that is translated into an
HTTP 400 response. The script converts itself properly and is
available at runtime.

Relates to #59332

This adds support for search time mappings to the _search API like so: ``` curl -XPOST -HContent-Type:application/json -uelastic:password 'localhost:9200/test/_search?pretty&error_trace' -d'{ "mappings": { "properties": { "cat": { "type": "script", "runtime_type": "keyword", "script": "value(\"cat\")" } } }, "docvalue_fields": ["cat"] }' ``` It just lifts the `mappings` field from the create index and mappings update requests and drops it into the `_search` request. It then gives each mapped field a chance to convert itself into a runtime field. Most mapping can not and will throw an exception that is translated into an HTTP 400 response. The `script` converts itself properly and is available at runtime.

nik9000 · 2020-07-09T20:58:45Z

"mappings": {
"properties": {

@javanna and I talked about changing it to just runtime_mappings. I used this syntax here because it is convenient, but it is, like the TODO in the code says, too big.

javanna

I left some comments, I guess one thing to look at is whether we could parse/validate the new section in the coordinating node so that we could fail early and once if something is wrong or if a non-runtime field is defined as part of the search request. Could we also validate the script once? I'm afraid it will be tough because MapperService is an index component, but there may be other ways to only validate?

javanna · 2020-07-09T19:36:42Z

server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

@@ -239,6 +244,9 @@ public SearchSourceBuilder(StreamInput in) throws IOException {
        sliceBuilder = in.readOptionalWriteable(SliceBuilder::new);
        collapse = in.readOptionalWriteable(CollapseBuilder::new);
        trackTotalHitsUpTo = in.readOptionalInt();
+        if (in.getVersion().onOrAfter(Version.V_8_0_0)) {


add a TODO to update the version once the branch is merged?

javanna · 2020-07-09T19:37:14Z

server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

+        if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+            out.writeMap(extraMapping);
+        } else {
+            throw new IllegalArgumentException(


why throw exception? shouldn't we rather not write the map, and that's it?

If we don't write the map then nodes that can't understand will silently ignore the fields which feels bad!

oh true, so only some nodes may have the runtime fields which messes up the results? This is pretty important indeed. I wonder what effect does throwing here makes from a user's point of view.

also is not completely the truth. it depends on the nodes that get hit?

The last I checked it'll cause the request to 400 with the message. It does depend on which node gets hit, yeah.

I think it'll also come up in CCS where I expect the remote cluster could be at an older version for longer. I'll update the error message a little.

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java

server/src/main/java/org/elasticsearch/index/mapper/Mapper.java

server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

nik9000 · 2020-07-14T00:03:49Z

Just added an integration test! Yay!

elasticmachine · 2020-07-14T10:59:32Z

Pinging @elastic/es-search (:Search/Search)

javanna

I left some comments/questions

server/src/main/java/org/elasticsearch/index/IndexService.java

server/src/main/java/org/elasticsearch/index/mapper/Mapper.java

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

javanna · 2020-07-14T14:54:20Z

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

+            Mapper.Builder<?> builder = parser.parse(runtimeEntry.getKey(), definition, documentMapperParser().parserContext());
+            Mapper mapper = builder.build(builderContext);
+
+            MapperUtils.collect(mapper, objectMappers, fieldMappers, fieldAliasMappers);


Can you expand on why we need this? I was expecting that alias or objects would be caught anyways by your checks below, that only runtime fields can be defined.

This should allow us to collect the sub-mappers under objects. I do need to make sure there is a test for though.

server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java

javanna · 2020-08-13T13:47:41Z

I marked this stalled as we decided to postpone allowing to define runtime fields as part of the search request. We will get back to this later.

javanna reviewed Jul 10, 2020

View reviewed changes

javanna reviewed Jul 13, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java Outdated Show resolved Hide resolved

nik9000 added 2 commits July 13, 2020 15:42

Itr

dc70b42

Itr

1938a0e

nik9000 requested a review from javanna July 13, 2020 20:07

nik9000 added 5 commits July 13, 2020 19:52

Tests

9d8e4b3

Merge branch 'feature/runtime_fields' into search_time_mapping

5a44788

Parse plz

7346b8b

Precommit

4891bda

Fix test

77f35cb

javanna added the :Search/Search Search-related issues that do not fall into other categories label Jul 14, 2020

elasticmachine added the Team:Search Meta label for search team label Jul 14, 2020

javanna reviewed Jul 14, 2020

View reviewed changes

nik9000 added 5 commits July 14, 2020 10:57

Fix test

2011791

Revert some stuff

2b9b34c

Revert

0546dc2

WIP

a86ffaa

Merge branch 'feature/runtime_fields' into search_time_mapping

87bf5bf

javanna mentioned this pull request Jul 15, 2020

Add support for runtime fields #59332

Closed

30 tasks

nik9000 added 7 commits July 21, 2020 10:52

Merge branch 'feature/runtime_fields' into search_time_mapping

8aeab2f

itr

3c12ed0

iter

09aa1f4

Tests

65a5326

Merge branch 'feature/runtime_fields' into search_time_mapping

27628d9

Oops

acefc5d

sub-objects

394ba8f

javanna added the stalled label Aug 13, 2020

javanna closed this Sep 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Teach _search to read search time mappings #59316

Teach _search to read search time mappings #59316

nik9000 commented Jul 9, 2020 •

edited

Loading

nik9000 commented Jul 9, 2020

javanna left a comment

javanna Jul 9, 2020

nik9000 Jul 12, 2020

javanna Jul 9, 2020

nik9000 Jul 12, 2020

javanna Jul 13, 2020

javanna Jul 13, 2020

nik9000 Jul 13, 2020

nik9000 commented Jul 14, 2020

elasticmachine commented Jul 14, 2020

javanna left a comment

javanna Jul 14, 2020

nik9000 Jul 14, 2020

javanna commented Aug 13, 2020

Teach _search to read search time mappings #59316

Teach _search to read search time mappings #59316

Conversation

nik9000 commented Jul 9, 2020 • edited Loading

nik9000 commented Jul 9, 2020

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Jul 14, 2020

elasticmachine commented Jul 14, 2020

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna commented Aug 13, 2020

nik9000 commented Jul 9, 2020 •

edited

Loading