Skip to content

Conversation

@jimczi
Copy link
Contributor

@jimczi jimczi commented Dec 26, 2016

scaled_float should be used as FLOAT in aggregations but currently they are used as LONG.
This change fixes this issue and adds a simple it test for it.

Fixes #22350

`scaled_float` should be used as FLOAT in aggregations but currently they are used as LONG.
This change fixes this issue and adds a simple it test for it.
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

/**
* {@link ScaledFloatLeafFieldData#getDoubleValues()} transforms the raw long values in `scaled` floats.
*/
return NumericType.FLOAT;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that will change anything in practice, but I think the correct value would be DOUBLE?

@jimczi jimczi merged commit e7444f7 into elastic:master Dec 27, 2016
@jimczi jimczi deleted the scaled_float_cast branch December 27, 2016 08:23
jimczi added a commit that referenced this pull request Dec 27, 2016
`scaled_float` should be used as DOUBLE in aggregations but currently they are used as LONG.
This change fixes this issue and adds a simple it test for it.

Fixes #22350
@jimczi
Copy link
Contributor Author

jimczi commented Dec 27, 2016

Thanks @jpountz
This is now pushed to master and 5.2

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.

scaled_float are used as longs in aggregations

2 participants