Skip to content

Enhance Elasticsearch String field mapping #29312

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 1 commit into
base: main
Choose a base branch
from

Conversation

OmarHawk
Copy link
Contributor

@OmarHawk OmarHawk commented Apr 24, 2025

Fix #29311


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@OmarHawk OmarHawk marked this pull request as ready for review April 24, 2025 09:54
@OmarHawk OmarHawk marked this pull request as draft April 24, 2025 13:29
@OmarHawk
Copy link
Contributor Author

OmarHawk commented Apr 24, 2025

This change produces problems when having String fields exceeding the max length of keywords (e.g. TextBlob or String with maxlength > 256). In that case, the contents cannot be indexed anymore. So, I converted this PR back to a Draft.

@OmarHawk OmarHawk force-pushed the feature/improve-es-textfields-mapping branch from 4439b0d to cc06356 Compare April 24, 2025 13:56
@OmarHawk
Copy link
Contributor Author

I've added another param to the annotation that fixes the issue with String contents > 256 characters - this is also part of the standard mapping, and I had missed that out before

@OmarHawk OmarHawk marked this pull request as ready for review April 24, 2025 14:35
@mshima
Copy link
Member

mshima commented Apr 24, 2025

@OmarHawk we are a few days from v9 beta.
Do you think we should include in v8?

@OmarHawk
Copy link
Contributor Author

@OmarHawk we are a few days from v9 beta. Do you think we should include in v8?

No, this PR goes rather into the 'better developer experience' and is not a bugfix per se. So, I think, we don't need it in v8.

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.

[elasticsearch] Missing .keyword field mapping for String fields
2 participants