"geo_distance" parsing bugs #454
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
October 26, 2010 15:06
Those two are supposed to be equivalent:
distance: 12, unit: "mi"
vs
distance: "12", unit: "mi"
but they are not because of an underlying bug in the query parsing
code, providing non-equivalent behavior whether a number or a string
comes via JSON.
The added test files should be equivalent. Actually they
hit the same bug as change
Two tests for parsing "geo_distance" filter: distance/unit parameters
If an explicit unit is provided with "distance", the "unit" can be safely ignored, as it works as a fallback unit.
This mistake should have been caught by DistanceUnitTests. But the problem is that the tests in this file does not run during the execution of the test suite, and I don't have a clue why this is so.
The problem was that when "unit" was given, the conversion to miles was happening too early, which caused wrong computations. This change postpones this computation when one really knows which unit should be used.
|
great catch!, will push in a few..., for some reason adding @test to the DistanceUnitTests class makes it run, it looks like it ran when you run it (all green), but nothing actually runs as you noted, very misleading.... |
|
pushed to both master and 0.12 branch, thanks. |
williamrandolph
pushed a commit
to williamrandolph/elasticsearch
that referenced
this pull request
Jun 4, 2020
mindw
pushed a commit
to mindw/elasticsearch
that referenced
this pull request
Sep 5, 2022
…ull request elastic#454) MPC-4558: disable EKS services exposure through ALB fixed response * comment out ALB fixed response for EKS services exposure Approved-by: Andre Sodermans Approved-by: Can Yildiz
costin
pushed a commit
to costin/elasticsearch
that referenced
this pull request
Dec 19, 2022
This PR adds support for reading null values to the `ValuesSourceReaderOperator` so sparse fields can be extracted. It builds on the work done in PR elastic#400
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Oct 2, 2023
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These add a few more tests related to the parsing of "geo_distance" filter – which expose some minor bugs with the handling of "distance" / "unit" parameters.
Then two changes to fix DistanceUnit and GeoDistanceFilterParser classes.
A summary of the issues is: