Fix match query on a scaled_float property no longer matches for some values #17879
Conversation
kkewwei
requested review from
anasalkouz,
andrross,
ashking94,
bugmakerrrrrr,
Bukhtawar,
CEHENKLE,
cwperks,
dbwiddis,
gbbafna,
jainankitk,
kotwanikunal,
linuxpi,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
github-actions
bot
added
bug
kkewwei
force-pushed
the
scaled_float_not_match
branch
from
e8f0619 to
40380fb
Compare
|
❌ Gradle check result for 40380fb: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
kkewwei
force-pushed
the
scaled_float_not_match
branch
from
40380fb to
adbf317
Compare
|
❌ Gradle check result for adbf317: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
| @@ -229,7 +229,7 @@ public String typeName() { | |||
| @Override | |||
| public Query termQuery(Object value, QueryShardContext context) { | |||
| failIfNotIndexedAndNoDocValues(); | |||
| long scaledValue = Math.round(scale(value)); | |||
| long scaledValue = Math.round(parse(value) * scalingFactor); | |||
There was a problem hiding this comment.
Consistent with precision loss when indexing
There was a problem hiding this comment.
We can also change the code in indexing, but it will cause indexing performance regression.
Math.round(doubleValue * scalingFactor) -> Math.round(scale(value))
There was a problem hiding this comment.
This look good to me, it's better to extract this logic Math.round(parse(value) * scalingFactor) to a single place, so termQuery and parseCreateField for indexing just use that.
|
❌ Gradle check result for 7a2fa3f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
kkewwei
force-pushed
the
scaled_float_not_match
branch
2 times, most recently
from
2ba7c47 to
ade3cae
Compare
|
❌ Gradle check result for 66bb50e: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
MinimumClusterManagerNodesIT.testThreeNodesNoClusterManagerBlock #14289 |
|
@bowenlan-amzn would you mind have a look in your spare time, very thank you. |
kkewwei
force-pushed
the
scaled_float_not_match
branch
from
66bb50e to
75cea4e
Compare
|
❌ Gradle check result for 75cea4e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
kkewwei
force-pushed
the
scaled_float_not_match
branch
from
75cea4e to
88bcdda
Compare
|
❌ Gradle check result for 88bcdda: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
EhCacheDiskCacheTests.testComputeIfAbsentConcurrently #16159 |
|
❌ Gradle check result for b1a19aa: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
… values Signed-off-by: kkewwei <[email protected]> Signed-off-by: kkewwei <[email protected]>
kkewwei
force-pushed
the
scaled_float_not_match
branch
from
b1a19aa to
b36c641
Compare
|
❌ Gradle check result for b36c641: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b36c641: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
RemoteStorePinnedTimestampsGarbageCollectionIT.testLiveIndexNoPinnedTimestampsWithExtraGenSetting #16088 |
Description
This PR only addresses the mismatch issue.
As it's known,
floatanddoubletypes suffer from precision loss, and it's inevitable. When readingdoc_valuein the filed(such as range, sorting, or aggregation), it can lead to unforeseen exceptions. The root cause lies in the fact that the precision of float/double data is already lost before storing.OpenSearch/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java
Line 468 in 9db5e67
It can be extremely terrifying when related to money.
To solve it, I will submit a new PR. In this PR, I will create a new type
big_decimal, supported by BigDecimal to effectively eliminate the precision loss.Related Issues
Resolves #12433
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.