-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Check that sort fields on all indices are compatible #74190
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/es-search (Team:Search) |
Is a different solution being considered for this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @scampi, thanks for working on this issue and apologies for the late response, this somehow fell through the cracks.
After taking a look at the issue in #73146 and the proposed fix now, I'm afraid that the proposed solution of comparing DocValueFormats Writable name might break some sort behaviour between types that currently works, while it might not be enough to prevent ClassCast exceptions for other types.
For example sorting across “keyword” and “version” type entries in different indices currently works, although the result might not always be expected (the special sorting of version fields is most likely not honoured), while e.g. mixing “double” and “float” numeric types still leads to a “class_cast_exception”, so it isn’t caught earlier by the modification to the
“validateMergeSortValueFormats” method.
I’m currently unsure where the best place to fix this would be and what kind of sorting behaviour between different mapping types we want to support, but I’ll hopefully get some info on this soon. Will either reply here or on the original issue.
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
Close #73146