Skip to content

Fix docvalue fetch for scaled floats #62425

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 16, 2020
Merged

Fix docvalue fetch for scaled floats #62425

merged 2 commits into from
Sep 16, 2020

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Sep 15, 2020

In #61995 I moved the docvalue_field fetch code into a place where I
could share it with the fancy new fields fetch API. Specifically,
runtime fields can use it all that doc values code now. But I broke
scaled_floats by switching them how they are fetched from double to
string. This adds the override you need to switch them back.

@nik9000
Fix docvalue fetch for scaled floats
78eff1f 
In elastic#61995 I moved the `docvalue_field` fetch code into a place where I
could share it with the fancy new `fields` fetch API. Specifically,
runtime fields can use it all that doc values code now. But I broke
`scaled_floats` by switching them how they are fetched from `double` to
`string`. This adds the override you need to switch them back.
@nik9000 nik9000 added >non-issue :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.10.0 labels Sep 15, 2020
@nik9000 nik9000 requested a review from javanna September 15, 2020 22:05
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Sep 15, 2020
@nik9000 nik9000 removed the v7.10.0 label Sep 15, 2020
@nik9000
Copy link
Member Author

nik9000 commented Sep 15, 2020

This fixed an unreleased bug so I've marked it non-issue. Since the build is broken without this and the code is in the same vein as #61995 I'm going to merge this when CI is happy with it. I'll also merge it into #61995's backport.

@nik9000 nik9000 merged commit 8a9028c into elastic:master Sep 16, 2020
javanna
javanna reviewed Sep 16, 2020
View reviewed changes
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

thanks Nik, and I thought I double checked that all the field types still worked correctly :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javanna javanna javanna left review comments

Assignees
No one assigned
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nik9000 @elasticmachine @javanna @jakelandis