-
Notifications
You must be signed in to change notification settings - Fork 99
Add composite fields to search API #2070
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
Conversation
Sorry for the delay! There were two changes in this pull request:
I merged main into this branch, so now there's only the second change left, and we need to review it. |
So I studied composite runtime fields, introduced in elastic/elasticsearch#75108. (Note that they're totally different from composite aggregations, despite sharing the same name.) The point of a composite runtime field is to have one script which emits multiple values, and place those values in different fields: https://www.elastic.co/guide/en/elasticsearch/reference/8.15/runtime-examples.html#runtime-examples-grok-composite. It looks like this: PUT my-index-000001/_mappings
{
"runtime": {
"http": {
"type": "composite",
"script": "emit(grok(\"%{COMMONAPACHELOG}\").extract(doc[\"message\"].value))",
"fields": {
"clientip": {
"type": "ip"
},
"verb": {
"type": "keyword"
},
"response": {
"type": "long"
}
}
}
}
} The possible types, as correctly identified by @MichaelOpitz2, are the composite field types. However, one question remained: is it possible to use something else than "type"? The code is hard to read because the code to read the runtime fields is reused for composite fields, but without allowing fields deeper than one level. Also, whenever I tried using something else than type, I got errors. Two examples:
And finally, YAML tests about composite runtime fields only use type: see 110_composite.yml and 114_composite_errors.yml. (We don't currently use these files for validation.) My conclusion is that only "type" is allowed, so I pushed a commit to clarify that. |
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!
Closes #2059, closes #2812
Re-add composite runtime fields according to spec: Runtime fields
The spec supports composite runtime fields sinde 7.15 but it seems they never made it into the new clients.
I am not sure, if these changes need to be backported, at least, composite runtime fields do not work in every version of the generated clients. I suppose number three applies: API is partially usable and the fix unlocks a missing feature that has no workaround