Skip to content

Build the correct keyword field type for PotentiallyUnmappedKeywordEsField #129854

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

martijnvg
Copy link
Member

Before a KeywordFieldType was created that didn't set the isSyntheticSource field, causing to use the wrong block loader that would synthesize the complete _source instead of just loading values from ignored source. This PR addresses this.

…Field

Before a `KeywordFieldType` was created that didn't set the isSyntheticSource field, causing to use the wrong block loader that would synthesize the complete _source instead of just loading values from ignored source. This PR addresses this.
@martijnvg martijnvg added >non-issue :Analytics/Compute Engine Analytics in ES|QL :StorageEngine/Mapping The storage related side of mappings labels Jun 23, 2025
@martijnvg martijnvg marked this pull request as ready for review June 23, 2025 14:11
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine labels Jun 23, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@@ -187,9 +197,22 @@ private static class DefaultShardContextForUnmappedField extends DefaultShardCon
@Override
public @Nullable MappedFieldType fieldType(String name) {
var superResult = super.fieldType(name);
return superResult == null && name.equals(unmappedEsField.getName())
? new KeywordFieldMapper.KeywordFieldType(name, false /* isIndexed */, false /* hasDocValues */, Map.of() /* meta */)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we remove or hide this constructor? It's asking for trouble..

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, let's do that in a different change? I see there're 48 usages in test code of this constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Compute Engine Analytics in ES|QL >non-issue :StorageEngine/Mapping The storage related side of mappings Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants