GEOMESA-1150 Query planning for some literal points is broken #851
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
The utility that computes Geohash substrings within a geometry now promotes the incoming geometry to a non-zero area figure. In addition, the FilterHelper has been refactored to handle POINT literal geometries in the right-hand-child slot. Unit tests were expanded to reproduce the originally observed error, and to confirm that those same tests now pass. Signed-off-by: Chris Eichelberger <cne1x@ccri.com>
| val stQueriedCount = fs.getFeatures(q).size | ||
| val indexOnlyCount = fs.getFeatures(indexOnlyQuery).size | ||
| //TODO cne1x debug! | ||
| println(s"Query:\n $filterString\n Expected count: $optExpectedCount -> $expectedCount" + |
There was a problem hiding this comment.
fix println (logger or remove)
| @@ -50,7 +67,9 @@ object FilterHelper { | |||
| updateToIDLSafeFilter(op, safeGeometry, featureType) | |||
| } | |||
|
|
|||
| // TODO: We assume "BINOP(property, geom)"... This need not be the case. | |||
There was a problem hiding this comment.
can reference the new ticket here (1155)
| case _ => | ||
| throw new Exception(s"Neither child of a binary spatial operator was a literal geometry: $op") | ||
| } | ||
| } |
There was a problem hiding this comment.
This is good for now. We have some code that does something similar here: org.locationtech.geomesa.filter#checkOrder. When we go back to evaluate GEOMESA-1155 we can revisit.
|
looks good to me |
| @@ -864,6 +864,18 @@ object GeohashUtils | |||
| } yield newStr).take(maxSize + 1) | |||
| } | |||
|
|
|||
| def promoteToRegion(geom: Geometry): Geometry = geom match { | |||
| case g: Point => | |||
| g.buffer(1e-6) | |||
There was a problem hiding this comment.
As an insane corner case, what happens if the Point to 'contain' is at one of the 4 'corners' of the world?
Just a quick thought experiment. Probably worth merging...
|
merging |
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.
The utility that computes Geohash substrings within a geometry now
promotes the incoming geometry to a non-zero area figure. In
addition, the FilterHelper has been refactored to handle POINT
literal geometries in the right-hand-child slot. Unit tests
were expanded to reproduce the originally observed error, and
to confirm that those same tests now pass.
Signed-off-by: Chris Eichelberger cne1x@ccri.com