-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Add runtime_mappings
to search request
#64374
Conversation
72b7768
to
706da74
Compare
Draft because I expect @javanna and I to do a fair bit of iteration on this before we get it in. This doesn't work with the |
This adds a way to specify the `runtime_mappings` on a search request which are always "runtime" fields. It looks like: ``` curl -XDELETE -uelastic:password -HContent-Type:application/json localhost:9200/test curl -XPOST -uelastic:password -HContent-Type:application/json 'localhost:9200/test/_bulk?pretty&refresh' -d' {"index": {}} {"animal": "cat", "sound": "meow"} {"index": {}} {"animal": "dog", "sound": "woof"} {"index": {}} {"animal": "snake", "sound": "hisssssssssssssssss"} ' curl -XPOST -uelastic:password -HContent-Type:application/json localhost:9200/test/_search?pretty -d' { "runtime_mappings": { "animal.upper": { "type": "keyword", "script": "for (String s : doc[\"animal.keyword\"]) {emit(s.toUpperCase())}" } }, "query": { "match": { "animal.upper": "DOG" } } }' ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @nik9000 I think that this is how it should look. It requires a bit more refactoring to make sure that when we lookup field types in the fetch phase, we use the method that knows about runtime mappings. I am on it, I think.
if (oldRuntimeType != null) { | ||
throw new ElasticsearchParseException("use [type] in [runtime_mappings] instead of [runtime_type]"); | ||
} | ||
runtimeMappings.put(field, buildFieldType("runtime", field, node, parserContextSupplier.get(), indexSettings)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice tricks! ;)
@@ -233,7 +235,8 @@ protected MultiSearchResponse shardOperation(Request request, ShardId shardId) t | |||
shardId.id(), | |||
searcher, | |||
() -> { throw new UnsupportedOperationException(); }, | |||
null | |||
null, | |||
emptyMap() // NOCOMMIT is it right not to use the runtime mappings? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I looked I think that this one gets a search source builder, so it seemed that runtime mappings can be specified, hence we should use them?
@javanna and I talked about this offline - I believe we'll have to modify the method that looks like |
66d854a
to
19f9746
Compare
I've resolved the |
server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java
Outdated
Show resolved
Hide resolved
What do we expect to happen when folks do this:
Right now you get a kind of silly error. I can make the error nicer. Or I could try and make it work? |
@nik9000 users won't be able to create objects in the runtime mappings, but rather only runtime fields, eventually with fields in their names. If you throw an error now, that's good enough. With the introduction of the runtime section the parsers for runtime fields will be separate from the others hence this will be handled there. |
@nik9000 maybe also mention overriding/shadowing in the description of the PR? For instance an example of shadowing a field within an object would help I think |
🤘 I'll make the error message reasonable and we can see what happens when we merge the runtime sections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @nik9000 !
In #64374 I broke a test that serializes `SearchRequest` with a random version. I'm unsure how we didn't catch this in the PR tests but computers are tricky. This fixes the test by removing runtime mappings for versions that don't support it.
This adds a way to specify the `runtime_mappings` on a search request which are always "runtime" fields. It looks like: ``` curl -XDELETE -uelastic:password -HContent-Type:application/json localhost:9200/test curl -XPOST -uelastic:password -HContent-Type:application/json 'localhost:9200/test/_bulk?pretty&refresh' -d' {"index": {}} {"animal": "cat", "sound": "meow"} {"index": {}} {"animal": "dog", "sound": "woof"} {"index": {}} {"animal": "snake", "sound": "hisssssssssssssssss"} ' curl -XPOST -uelastic:password -HContent-Type:application/json localhost:9200/test/_search?pretty -d' { "runtime_mappings": { "animal.upper": { "type": "keyword", "script": "for (String s : doc[\"animal.keyword\"]) {emit(s.toUpperCase())}" } }, "query": { "match": { "animal.upper": "DOG" } } }' ``` NOTE: If we have to send a search request with runtime mappings to a node that doesn't support runtime mappings at all then we'll fail the search request entirely. The alternative would be to not send those runtime mappings and let the node fail the search request with an "unknown field" error. I believe this is would be hard to surprising because you defined the field in the search request. NOTE: It isn't obvious but you can also use `runtime_mappings` to override fields inside objects by naming the runtime fields with `.` in them. Like this: ``` curl -XDELETE -uelastic:password -HContent-Type:application/json localhost:9200/test curl -uelastic:password -XPOST -HContent-Type:application/json localhost:9200/test/_bulk?refresh -d' {"index":{}} {"name": {"first": "Andrew", "last": "Wiggin"}} {"index":{}} {"name": {"first": "Julian", "last": "Delphiki", "suffix": "II"}} ' curl -uelastic:password -XPOST -HContent-Type:application/json localhost:9200/test/_search?pretty -d'{ "runtime_mappings": { "name.first": { "type": "keyword", "script": "if (\"Wiggin\".equals(doc[\"name.last.keyword\"].value)) {emit(\"Ender\");} else if (\"Delphiki\".equals(doc[\"name.last.keyword\"].value)) {emit(\"Bean\");}" } }, "query": { "match": { "name.first": "Bean" } } }' ``` Relates to elastic#59332
Prepare to backport elastic#64374 by updating some versions constants so we can send `runtime_mappings` to 7.11.0. Also disable bwc tests so they don't fail until we finish the backport.
Prepare to backport #64374 by updating some versions constants so we can send `runtime_mappings` to 7.11.0. Also disable bwc tests so they don't fail until we finish the backport.
* Add `runtime_mappings` to search request (backport of #64374) This adds a way to specify the `runtime_mappings` on a search request which are always "runtime" fields. It looks like: ``` curl -XDELETE -uelastic:password -HContent-Type:application/json localhost:9200/test curl -XPOST -uelastic:password -HContent-Type:application/json 'localhost:9200/test/_bulk?pretty&refresh' -d' {"index": {}} {"animal": "cat", "sound": "meow"} {"index": {}} {"animal": "dog", "sound": "woof"} {"index": {}} {"animal": "snake", "sound": "hisssssssssssssssss"} ' curl -XPOST -uelastic:password -HContent-Type:application/json localhost:9200/test/_search?pretty -d' { "runtime_mappings": { "animal.upper": { "type": "keyword", "script": "for (String s : doc[\"animal.keyword\"]) {emit(s.toUpperCase())}" } }, "query": { "match": { "animal.upper": "DOG" } } }' ``` NOTE: If we have to send a search request with runtime mappings to a node that doesn't support runtime mappings at all then we'll fail the search request entirely. The alternative would be to not send those runtime mappings and let the node fail the search request with an "unknown field" error. I believe this is would be hard to surprising because you defined the field in the search request. NOTE: It isn't obvious but you can also use `runtime_mappings` to override fields inside objects by naming the runtime fields with `.` in them. Like this: ``` curl -XDELETE -uelastic:password -HContent-Type:application/json localhost:9200/test curl -uelastic:password -XPOST -HContent-Type:application/json localhost:9200/test/_bulk?refresh -d' {"index":{}} {"name": {"first": "Andrew", "last": "Wiggin"}} {"index":{}} {"name": {"first": "Julian", "last": "Delphiki", "suffix": "II"}} ' curl -uelastic:password -XPOST -HContent-Type:application/json localhost:9200/test/_search?pretty -d'{ "runtime_mappings": { "name.first": { "type": "keyword", "script": "if (\"Wiggin\".equals(doc[\"name.last.keyword\"].value)) {emit(\"Ender\");} else if (\"Delphiki\".equals(doc[\"name.last.keyword\"].value)) {emit(\"Bean\");}" } }, "query": { "match": { "name.first": "Bean" } } }' ``` Relates to #59332
Now that elastic#64374 has been backported to 7.x we can run the bwc tests again without wire incompatibilities.
Now that #64374 has been backported to 7.x we can run the bwc tests again without wire incompatibilities.
This adds a way to specify the
runtime_mappings
on a search requestwhich are always "runtime" fields. It looks like:
NOTE:
If we have to send a search request with runtime mappings to a node that
doesn't support runtime mappings at all then we'll fail the search
request entirely. The alternative would be to not send those runtime
mappings and let the node fail the search request with an "unknown field"
error. I believe this is would be hard to surprising because you defined
the field in the search request.
NOTE:
It isn't obvious but you can also use
runtime_mappings
to override fieldsinside objects by naming the runtime fields with
.
in them. Like this:Relates to #59332