Skip to content

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

Merged
merged 9 commits into from
Sep 5, 2024
Merged

Conversation

MichaelOpitz2
Copy link
Contributor

@MichaelOpitz2 MichaelOpitz2 commented Apr 12, 2023

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

@pquentin
Copy link
Member

Sorry for the delay! There were two changes in this pull request:

  1. Adding composite to RuntimeFieldType, which was done in Java client issues batch 3 #2485 and was applied to all 8.13 clients
  2. Adding fields?: Dictionary<string, Dictionary<string, RuntimeFieldType>> to RuntimeField

I merged main into this branch, so now there's only the second change left, and we need to review it.

@pquentin
Copy link
Member

pquentin commented Sep 5, 2024

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:

  • Found sub-fields with name not belonging to the parent field they are part of [response]
  • Failed to parse mapping: Cannot use [script] parameter on sub-field [verb] of composite field [http]

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.

Copy link
Contributor

@l-trotta l-trotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pquentin pquentin merged commit 84f7a33 into elastic:main Sep 5, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interface for RuntimeField missing string option
3 participants