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

Int and Float field types should use their corresponding Sort types #84799

Open
romseygeek opened this issue Mar 9, 2022 · 4 comments
Open
Labels
>enhancement :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch >tech debt

Comments

@romseygeek
Copy link
Contributor

Description

The int and float field types in Elasticsearch use long and double sort types. This mismatch means that we can't make use of lucene's sorting shortcuts for these field types.

A complication here is that just changing the sort types used at runtime may run into problems when dealing with sorted indexes, which will have already written the old, wider sort type into their index metadata.

@romseygeek romseygeek added >enhancement :Search/Search Search-related issues that do not fall into other categories labels Mar 9, 2022
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 9, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@LeeTgk
Copy link

LeeTgk commented May 25, 2022

Hi. I would like to try working on this issue.

@benwtrent
Copy link
Member

Reading a little into this, I tracked down places where this logic needs updated.

First, we need to create new set SortField objects that provide FieldComparator objects that appropriately transform between primitive types. Meaning, for comparing float and double, the objects are casted appropriately.

Second, SearchPhaseController needs to have updated logic for checkSameSortTypes and how it builds the corresponding sort object. Instead of blindly taking the first docs group and using those sorts (since they might be of Float, but you also have Double), it should return one of the new SortField objects if we determine we have many different, but compatible types.

Third, IndexNumericFieldData.NumericType needs updated so that the appropriate primitive type is provided.

There are probably other refactors required, but these are what I found so far.

@javanna javanna added :Search Relevance/Search Catch all for Search Relevance and removed :Search/Search Search-related issues that do not fall into other categories labels Jul 17, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Jul 17, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine elasticsearchmachine removed the Team:Search Meta label for search team label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch >tech debt
Projects
None yet
Development

No branches or pull requests

6 participants