Skip to content

fix 'runtime' property type in DynamicTemplate class #3847

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

Conversation

fbaligand
Copy link
Contributor

Currently, in DynamicTemplate class, the "runtime" property has "Property" type.
This PR aims to fix that by referencing "RuntimeField" type.

@fbaligand fbaligand changed the title fix 'runtime' field type in DynamicTemplate class fix 'runtime' property type in DynamicTemplate class Feb 26, 2025
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

This seems correct to me. We don't have any YAML tests on this, only Java tests (that should probably have been YAML tests.) runtime, like mapping is parsed as a Map<String, Object>, which isn't helpful:

https://github.com/elastic/elasticsearch/blob/1e8262170ed339d8f2ebdc6de6fd581adb40096d/server/src/main/java/org/elasticsearch/index/mapper/DynamicTemplate.java#L293-L301

But there's a unit test that shows that runtime is parsed as RuntimeField:

https://github.com/elastic/elasticsearch/blob/1e8262170ed339d8f2ebdc6de6fd581adb40096d/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java#L609-L642

This is what the mappings look like in JSON:

"dynamic_templates": [
  {
    "my_template": {
      "match_mapping_type": "*",
      "runtime": {
        "type": "{dynamic_type}",
        "foo": "bar"
      }
    }
  }
]

And the error is unknown parameter [foo] on runtime field [__dynamic__my_template] of type [date] which is raised by the RuntimeField class:

https://github.com/elastic/elasticsearch/blob/1e8262170ed339d8f2ebdc6de6fd581adb40096d/server/src/main/java/org/elasticsearch/index/mapper/RuntimeField.java#L82-L86

LGTM!

@pquentin pquentin merged commit 86bbcf8 into elastic:main Feb 26, 2025
10 of 11 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 26, 2025
github-actions bot pushed a commit that referenced this pull request Feb 26, 2025
@fbaligand fbaligand deleted the fix/runtime_field_in_dynamic_template branch February 26, 2025 09:17
@fbaligand
Copy link
Contributor Author

Thanks for the merge @pquentin !

pquentin pushed a commit that referenced this pull request Feb 26, 2025
(cherry picked from commit 86bbcf8)

Co-authored-by: Fabien Baligand <[email protected]>
pquentin pushed a commit that referenced this pull request Feb 26, 2025
(cherry picked from commit 86bbcf8)

Co-authored-by: Fabien Baligand <[email protected]>
github-actions bot pushed a commit that referenced this pull request Feb 26, 2025
@pquentin
Copy link
Member

Merci pour la contribution !

This will be released in version 8.18 and above of the Elasticsearch clients.

pquentin pushed a commit that referenced this pull request Feb 26, 2025
(cherry picked from commit 86bbcf8)

Co-authored-by: Fabien Baligand <[email protected]>
@fbaligand
Copy link
Contributor Author

Super!

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.

2 participants