Skip to content
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

Updated the SparseVectorStrategy class to use sparse_vector query #2657

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

miguelgrinberg
Copy link
Contributor

This change updates the SparseVectorStrategy class to use the newer sparse_vector query instead of text_expansion, which generates a deprecation warning since 8.15.

Copy link

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

@joemcelroy
Copy link
Member

The complication with this issue is that sparse_vector query is not compatible with the rank_features mapping. This change will work perfectly for those who setup the index with this change (creates a sparse_vector type field) but will impact those who upgrade and still using rank_features mapping.

Possible workarounds for this is introduce a boolean to allow the "legacy" text_expansion query to be used in the strategy constructor. By default its the sparse_vector query. Add a better error to surface this configuration when ES responds with 400 error that they need to use the legacy boolean. Pain points is it will be a breaking change on upgrade.

@miguelgrinberg
Copy link
Contributor Author

@joemcelroy Discussing this within our team we were thinking that we should hold off on this change until 9.0, where we would have more freedom to introduce it as a breaking change.

Another option would be to have both the old and the new solutions in the client, and select which one to use based on the version reported by the server. But I don't think this would address a case where an index was created while running a <= 8.14 stack, but then the stack is upgraded to >= 8.15.

@joemcelroy
Copy link
Member

Yeh think thats wise to wait for 9.x. Theres nothing pressing to make this change, just the legacy warning.

But I don't think this would address a case where an index was created while running a <= 8.14 stack, but then the stack is upgraded to >= 8.15.

Yep thats it. Also will impact those who haven't pinned their ES client to their version of ES.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants