Skip to content

ESQL: Fix precision of scaled_float field values retrieved from stored source #122586

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

Merged
lkts merged 9 commits into from
Feb 20, 2025
Merged

ESQL: Fix precision of scaled_float field values retrieved from stored source #122586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c59225f
Consistency in using scaled_float field
kanoshiou Feb 14, 2025
1646ecc
Fix failed testcases
kanoshiou Feb 14, 2025
bdfb204
Update changelog
kanoshiou Feb 15, 2025
870b667
Merge branch 'main' into consistency-in-scaled_float-field
kanoshiou Feb 15, 2025
33ccb7c
Merge branch 'main' into consistency-in-scaled_float-field
kanoshiou Feb 19, 2025
44e508f
Backward compatibility
kanoshiou Feb 19, 2025
e8c50c4
Merge branch 'main' into consistency-in-scaled_float-field
lkts Feb 20, 2025
61a190e
Apply capability in one more test
lkts Feb 20, 2025
500c882
Merge branch 'main' into consistency-in-scaled_float-field
lkts Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/122586.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 122586
summary: "ESQL: Fix inconsistent results in using scaled_float field"
area: ES|QL
type: bug
issues:
- 122547
3 changes: 1 addition & 2 deletions ...er-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
return BlockLoader.CONSTANT_NULLS;
}
if (hasDocValues()) {
double scalingFactorInverse = 1d / scalingFactor;
return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l * scalingFactorInverse);
return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l / scalingFactor);
}
if (isSyntheticSource) {
return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
Expand Down
10 changes: 0 additions & 10 deletions ...src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldBlockLoaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,6 @@ public ScaledFloatFieldBlockLoaderTests() {
protected Double convert(Number value, Map<String, Object> fieldMapping) {
var scalingFactor = ((Number) fieldMapping.get("scaling_factor")).doubleValue();

var docValues = (boolean) fieldMapping.getOrDefault("doc_values", false);

// There is a slight inconsistency between values that are read from doc_values and from source.
// Due to how precision reduction is applied to source values so that they are consistent with doc_values.
// See #122547.
if (docValues) {
var reverseScalingFactor = 1d / scalingFactor;
return Math.round(value.doubleValue() * scalingFactor) * reverseScalingFactor;
}

// Adjust values coming from source to the way they are stored in doc_values.
// See mapper implementation.
return Math.round(value.doubleValue() * scalingFactor) / scalingFactor;
Expand Down
4 changes: 1 addition & 3 deletions .../plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,6 @@ private static StringBuilder trimOrPad(StringBuilder buffer) {

private static double scaledFloat(String value, String factor) {
double scalingFactor = Double.parseDouble(factor);
// this extra division introduces extra imprecision in the following multiplication, but this is how ScaledFloatFieldMapper works.
double scalingFactorInverse = 1d / scalingFactor;
return new BigDecimal(value).multiply(BigDecimal.valueOf(scalingFactor)).longValue() * scalingFactorInverse;
return new BigDecimal(value).multiply(BigDecimal.valueOf(scalingFactor)).longValue() / scalingFactor;
}
}
20 changes: 12 additions & 8 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,39 @@ a:double | b:double
;

inDouble
required_capability: fix_precision_of_scaled_float_fields
from employees | keep emp_no, height, height.float, height.half_float, height.scaled_float | where height in (2.03, 2.0299999713897705, 2.029296875, 2.0300000000000002) | sort emp_no;

emp_no:integer |height:double |height.float:double |height.half_float:double |height.scaled_float:double
10001 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10090 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10001 |2.03 |2.0299999713897705 |2.029296875 |2.03
Copy link
Contributor

@lkts lkts Feb 18, 2025

Choose a reason for hiding this comment

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

You need to add a new capability for this fix and use it in this test similar to how fix_parsing_large_negative_numbers is used above. This is needed to fix backward compatibility tests.

10090 |2.03 |2.0299999713897705 |2.029296875 |2.03
;

inFloat
required_capability: fix_precision_of_scaled_float_fields
from employees | keep emp_no, height, height.float, height.half_float, height.scaled_float | where height.float in (2.03, 2.0299999713897705, 2.029296875, 2.0300000000000002) | sort emp_no;

emp_no:integer |height:double |height.float:double |height.half_float:double |height.scaled_float:double
10001 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10090 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10001 |2.03 |2.0299999713897705 |2.029296875 |2.03
10090 |2.03 |2.0299999713897705 |2.029296875 |2.03
;

inHalfFloat
required_capability: fix_precision_of_scaled_float_fields
from employees | keep emp_no, height, height.float, height.half_float, height.scaled_float | where height.half_float in (2.03, 2.0299999713897705, 2.029296875, 2.0300000000000002) | sort emp_no;

emp_no:integer |height:double |height.float:double |height.half_float:double |height.scaled_float:double
10001 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10090 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10001 |2.03 |2.0299999713897705 |2.029296875 |2.03
10090 |2.03 |2.0299999713897705 |2.029296875 |2.03
;

inScaledFloat
required_capability: fix_precision_of_scaled_float_fields
from employees | keep emp_no, height, height.float, height.half_float, height.scaled_float | where height.scaled_float in (2.03, 2.0299999713897705, 2.029296875, 2.0300000000000002) | sort emp_no;

emp_no:integer |height:double |height.float:double |height.half_float:double |height.scaled_float:double
10001 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10090 |2.03 |2.0299999713897705 |2.029296875 |2.0300000000000002
10001 |2.03 |2.0299999713897705 |2.029296875 |2.03
10090 |2.03 |2.0299999713897705 |2.029296875 |2.03
;

convertFromDouble
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,11 @@ height:double | languages.long:long | still_hired:boolean
;

simpleEvalWithSortAndLimitOne
required_capability: fix_precision_of_scaled_float_fields
from employees | eval x = languages + 7 | sort x, avg_worked_seconds | limit 1;

avg_worked_seconds:long | birth_date:date | emp_no:integer | first_name:keyword | gender:keyword | height:double | height.float:double | height.half_float:double | height.scaled_float:double | hire_date:date | is_rehired:boolean | job_positions:keyword | languages:integer | languages.byte:integer | languages.long:long | languages.short:integer | last_name:keyword | salary:integer | salary_change:double | salary_change.int:integer | salary_change.keyword:keyword |salary_change.long:long | still_hired:boolean | x:integer
208374744 |1956-11-14T00:00:00.000Z|10033 |null |M |1.63 |1.6299999952316284|1.6298828125 |1.6300000000000001 |1987-03-18T00:00:00.000Z|true |null |1 |1 |1 |1 |Merlo |70011 |null |null |null |null |false |8
208374744 |1956-11-14T00:00:00.000Z|10033 |null |M |1.63 |1.6299999952316284|1.6298828125 |1.63 |1987-03-18T00:00:00.000Z|true |null |1 |1 |1 |1 |Merlo |70011 |null |null |null |null |false |8
;

evalOfAverageValue
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/esql/qa/testFixtures/src/main/resources/mv_expand.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,11 @@ a | a

//see https://github.com/elastic/elasticsearch/issues/103331
keepStarMvExpand#[skip:-8.12.99]
required_capability: fix_precision_of_scaled_float_fields
from employees | where emp_no == 10001 | keep * | mv_expand first_name;

avg_worked_seconds:long | birth_date:date | emp_no:integer | first_name:keyword | gender:keyword | height:double | height.float:double | height.half_float:double | height.scaled_float:double | hire_date:date | is_rehired:boolean | job_positions:keyword | languages:integer | languages.byte:integer | languages.long:long | languages.short:integer | last_name:keyword | salary:integer | salary_change:double | salary_change.int:integer | salary_change.keyword:keyword | salary_change.long:long | still_hired:boolean
268728049 | 1953-09-02T00:00:00.000Z | 10001 | Georgi | M | 2.03 | 2.0299999713897705 | 2.029296875 | 2.0300000000000002 | 1986-06-26T00:00:00.000Z | [false, true] | [Accountant, Senior Python Developer] | 2 | 2 | 2 | 2 | Facello | 57305 | 1.19 | 1 | 1.19 | 1 | true
268728049 | 1953-09-02T00:00:00.000Z | 10001 | Georgi | M | 2.03 | 2.0299999713897705 | 2.029296875 | 2.03 | 1986-06-26T00:00:00.000Z | [false, true] | [Accountant, Senior Python Developer] | 2 | 2 | 2 | 2 | Facello | 57305 | 1.19 | 1 | 1.19 | 1 | true
;


Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/esql/qa/testFixtures/src/main/resources/rename.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,12 @@ y:integer | x:date
;

renameIntertwinedWithSort
required_capability: fix_precision_of_scaled_float_fields
FROM employees | eval x = salary | rename x as y | rename y as x | sort x | rename x as y | limit 10;

avg_worked_seconds:l | birth_date:date | emp_no:i | first_name:s | gender:s | height:d | height.float:d | height.half_float:d | height.scaled_float:d| hire_date:date | is_rehired:bool | job_positions:s | languages:i | languages.byte:i | languages.long:l | languages.short:i | last_name:s | salary:i | salary_change:d | salary_change.int:i | salary_change.keyword:s | salary_change.long:l | still_hired:bool | y:i

390266432 | 1959-08-19T00:00:00.000Z | 10015 | Guoxiang | null | 1.66 | 1.659999966621399 | 1.66015625 | 1.6600000000000001 | 1987-07-02T00:00:00.000Z | [false, false, false, true]| [Head Human Resources, Junior Developer, Principal Support Engineer, Support Engineer] | 5 | 5 | 5 | 5 | Nooteboom | 25324 | [12.4, 14.25] | [12, 14] | [12.40, 14.25] | [12, 14] | true | 25324
390266432 | 1959-08-19T00:00:00.000Z | 10015 | Guoxiang | null | 1.66 | 1.659999966621399 | 1.66015625 | 1.66 | 1987-07-02T00:00:00.000Z | [false, false, false, true]| [Head Human Resources, Junior Developer, Principal Support Engineer, Support Engineer] | 5 | 5 | 5 | 5 | Nooteboom | 25324 | [12.4, 14.25] | [12, 14] | [12.40, 14.25] | [12, 14] | true | 25324
203838153 | 1953-02-08T00:00:00.000Z | 10035 | null | M | 1.81 | 1.809999942779541 | 1.8095703125 | 1.81 | 1988-09-05T00:00:00.000Z | false | [Data Scientist, Senior Python Developer] | 5 | 5 | 5 | 5 | Chappelet | 25945 | [-6.58, -2.54] | [-6, -2] | [-2.54, -6.58] | [-6, -2] | false | 25945
313407352 | 1964-10-18T00:00:00.000Z | 10092 | Valdiodio | F | 1.75 | 1.75 | 1.75 | 1.75 | 1989-09-22T00:00:00.000Z | [false, false, true, true] | [Accountant, Junior Developer] | 1 | 1 | 1 | 1 | Niizuma | 25976 | [-6.77, 0.39, 8.3, 8.78] | [-6, 0, 8, 8] | [-6.77, 0.39, 8.30,8.78] | [-6, 0, 8, 8] | false | 25976
248451647 | null | 10048 | Florian | M | 2.0 | 2.0 | 2.0 | 2.0 | 1985-02-24T00:00:00.000Z | [true, true] | Internship | 3 | 3 | 3 | 3 | Syrotiuk | 26436 | null | null | null | null | false | 26436
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_count_distinct.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ h:long
;

countDistinctOfScaledFloat
required_capability: fix_precision_of_scaled_float_fields
// float becomes double until https://github.com/elastic/elasticsearch-internal/issues/724
from employees | stats h = count_distinct(height.scaled_float);

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,10 +734,11 @@ null
;

convertFromFloats
required_capability: fix_precision_of_scaled_float_fields
from employees | sort emp_no| eval double = to_string(height), float = to_string(height.float), scaled_float = to_string(height.scaled_float), half_float = to_string(height.half_float) | keep emp_no, double, float, scaled_float, half_float, height | limit 2;

emp_no:integer |double:keyword |float:keyword |scaled_float:keyword |half_float:keyword |height:double
10001 |2.03 |2.0299999713897705|2.0300000000000002 |2.029296875 |2.03
10001 |2.03 |2.0299999713897705|2.03 |2.029296875 |2.03
10002 |2.08 |2.0799999237060547|2.08 |2.080078125 |2.08
;

Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,12 @@ public enum Cap {
*/
FIX_PARSING_LARGE_NEGATIVE_NUMBERS,

/**
* Fix precision of scaled_float field values retrieved from stored source
* see <a href="https://github.com/elastic/elasticsearch/issues/122547"> Slight inconsistency in ESQL using scaled_float field #122547 </a>
*/
FIX_PRECISION_OF_SCALED_FLOAT_FIELDS,

/**
* Fix the status code returned when trying to run count_distinct on the _source type (which is not supported).
* see <a href="https://github.com/elastic/elasticsearch/issues/105240">count_distinct(_source) returns a 500 response</a>
Expand Down