-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Use safe double range to test long mapper #133423
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
Conversation
Until elastic#132893 is fixed this sounds like the best way to handle it.
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restricting the value range LGTM but I would like to wait for a comment by @nik9000 (also on the hard-coded test in #132893) of what the current expectation for long to double conversions are (especially also in the esql context). Just curious if we need more follow ups here or if it should be clear to everybody that conversions outside this range are not exact by nature.
Not sure, the test in #132893 seems to be very specific to test "bad" value, so I presume it was expected to work somehow? |
💔 Backport failed
You can use sqren/backport to manually backport by running |
Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
…33585) Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
…33585) Until elastic#132893 is fixed this sounds like the best way to handle it. (cherry picked from commit 8f42478) # Conflicts: # muted-tests.yml # server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java
Until #132893 is fixed this sounds like the best way to handle it.
Note: this does reduce some of the test functionality, but since we have the test above that tests it directly, we don't need to fail other tests for it. We can extend the range back once the test above is fixed.
Closes #132948
Closes #132956
Closes #132964
Closes #133394
Closes #133426