Skip to content
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

Adds geo-bounding box query documentation #2134

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

kolchfa-aws
Copy link
Collaborator

Fixes #1815

Checklist

  • [ x] By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
@kolchfa-aws kolchfa-aws self-assigned this Dec 5, 2022

### Using geohash to specify the bounding box

If you use a geohash to specify the bounding box, the geohash is treated as a rectangle. The upper left vertex of the bounding box corresponds to the upper left vertex of the `top_left` geohash, and the lower right vertex of the bounding box corresponds to the lower right vertex of the `bottom_right` geohash. To specify the bounding box that covers the whole area of a geohash, provide that geohash as both `top_left` and `bottom_right` parameters of the bounding box.
Copy link
Member

Choose a reason for hiding this comment

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

Shall we add an example for geo_hash?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added two examples in this section.

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
@VijayanB
Copy link
Member

VijayanB commented Dec 5, 2022

Overall looks good. one minor comment, shall we rename geopoint to geo_point wherever it is mentioned as field type.

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
@kolchfa-aws
Copy link
Collaborator Author

@VijayanB Thank you so much for the review! I added geohash examples. As for the field name, we refer to geopoints and geoshapes as one word without underscores (https://opensearch.org/docs/latest/opensearch/supported-field-types/geographic/).

@carolxob carolxob self-requested a review December 5, 2022 22:14
Copy link
Contributor

@carolxob carolxob left a comment

Choose a reason for hiding this comment

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

LGTM.

@kolchfa-aws kolchfa-aws marked this pull request as ready for review December 5, 2022 22:23
@kolchfa-aws kolchfa-aws requested a review from a team as a code owner December 5, 2022 22:23

You can use a geo-bounding box query to search for documents that contain geopoints.

Create a mapping with the `point` field is mapped as `geo_point`:
Copy link
Contributor

Choose a reason for hiding this comment

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

"Create a mapping where the point field is mapped as geo_point:"
Or,
"Create a mapping with the point field mapped as geo_point:"

}
```

The preceding response does not include document 3 with a geopoint of `"lat": 75.00, "lon": 28.00` because of the geopoint's limited [precision](#precision).
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it's worthwhile adding "... include document 3 (_doc/3) with a geopoint of ..." ?
Or, even just say "document #3"
It may not be necessary. But it took me a second to understand what that referred to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change to "the document with a geopoint...". The id is not too relevant.


## Precision

Geopoint coordinates are always rounded down at index time. At query time, the bounding box upper boundaries are rounded down, and the lower boundaries are rounded up. Therefore, the documents with geopoints that lie on the lower and left edges of the bounding box might not be included in the results due to rounding error. On the other hand, geopoints that lie on the upper and right edges of the bounding box might be included in the results even though they are outside the boundaries. The rounding error is less than 4.20 &times; 10<sup>&minus;8</sup> degrees for latitude and less than 8.39 &times; 10<sup>&minus;8</sup> degrees for longitude (around 1 cm).
Copy link
Contributor

Choose a reason for hiding this comment

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

"... due to a rounding error."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"due to rounding error" is a common phrase in computer science that usually doesn't have an article. Let's ask @natebower's opinion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on a little searching, it looks like "rounding error" refers to the concept, so "due to rounding error" is correct.

Copy link
Contributor

@cwillum cwillum left a comment

Choose a reason for hiding this comment

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

LGTM (with comments).

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@kolchfa-aws Please see my comments and changes and let me know if you have any questions. Thanks!

}
```

Search for all documents and filter those documents whose points lie within the rectangle defined in the query:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could "those" be "the"?


## Precision

Geopoint coordinates are always rounded down at index time. At query time, the bounding box upper boundaries are rounded down, and the lower boundaries are rounded up. Therefore, the documents with geopoints that lie on the lower and left edges of the bounding box might not be included in the results due to rounding error. On the other hand, geopoints that lie on the upper and right edges of the bounding box might be included in the results even though they are outside the boundaries. The rounding error is less than 4.20 &times; 10<sup>&minus;8</sup> degrees for latitude and less than 8.39 &times; 10<sup>&minus;8</sup> degrees for longitude (around 1 cm).
Copy link
Collaborator

Choose a reason for hiding this comment

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

"the upper boundaries of the bounding box"?


## Precision

Geopoint coordinates are always rounded down at index time. At query time, the bounding box upper boundaries are rounded down, and the lower boundaries are rounded up. Therefore, the documents with geopoints that lie on the lower and left edges of the bounding box might not be included in the results due to rounding error. On the other hand, geopoints that lie on the upper and right edges of the bounding box might be included in the results even though they are outside the boundaries. The rounding error is less than 4.20 &times; 10<sup>&minus;8</sup> degrees for latitude and less than 8.39 &times; 10<sup>&minus;8</sup> degrees for longitude (around 1 cm).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on a little searching, it looks like "rounding error" refers to the concept, so "due to rounding error" is correct.

- `top_right` and `bottom_left`
- `top`, `left`, `bottom`, and `right`

The following example specifies the bounding box using the `top`, `left`, `bottom`, and `right` coordinates:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The example doesn't itself specify the bounding box. "The following example shows how to specify the bounding box"?

:--- | :--- | :---
_name | String | The name of the filter. Optional.
validation_method | String | The validation method. Valid values are `IGNORE_MALFORMED` (accept geopoints with invalid coordinates), `COERCE` (try to coerce coordinates to valid values), and `STRICT` (return an error when coordinates are invalid). Default is `STRICT`.
type | String | Specifies how to execute the filter. Valid values are `indexed` (index the filter) and `memory` (execute the filter in memory). Default is `memory`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change "execute" to "run" or "apply" (or another appropriate verb).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think "execute" is the best verb here. Will keep it for clarity.


## Accepted formats

You can specify coordinates of the bounding box vertices in any [format]({{site.url}}{{site.baseurl}}/opensearch/supported-field-types/geo-point#formats) that geopoint accepts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The end of this sentence makes it sound as though "geopoint" is a proper noun. My brain wants it to be "that accepts geopoints." Can we revise for clarity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reworded.


You can specify coordinates of the bounding box vertices in any [format]({{site.url}}{{site.baseurl}}/opensearch/supported-field-types/geo-point#formats) that geopoint accepts.

### Using geohash to specify the bounding box
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Using a geohash" (to match the following sentence)?


### Using geohash to specify the bounding box

If you use a geohash to specify the bounding box, the geohash is treated as a rectangle. The upper left vertex of the bounding box corresponds to the upper left vertex of the `top_left` geohash, and the lower right vertex of the bounding box corresponds to the lower right vertex of the `bottom_right` geohash.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
If you use a geohash to specify the bounding box, the geohash is treated as a rectangle. The upper left vertex of the bounding box corresponds to the upper left vertex of the `top_left` geohash, and the lower right vertex of the bounding box corresponds to the lower right vertex of the `bottom_right` geohash.
If you use a geohash to specify the bounding box, the geohash is treated as a rectangle. The upper-left vertex of the bounding box corresponds to the upper-left vertex of the `top_left` geohash, and the lower-right vertex of the bounding box corresponds to the lower-right vertex of the `bottom_right` geohash.


If you use a geohash to specify the bounding box, the geohash is treated as a rectangle. The upper left vertex of the bounding box corresponds to the upper left vertex of the `top_left` geohash, and the lower right vertex of the bounding box corresponds to the lower right vertex of the `bottom_right` geohash.

The following example uses a geohash to specify the same bounding box as the previous examples:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The following example uses a geohash to specify the same bounding box as the previous examples:
The following example shows how to use a geohash to specify the same bounding box as in the previous examples:

}
```

To specify the bounding box that covers the whole area of a geohash, provide that geohash as both `top_left` and `bottom_right` parameters of the bounding box:
Copy link
Collaborator

Choose a reason for hiding this comment

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

"To specify a bounding box"? "the" is correct if there is just one preexisting bounding box that covers the whole area.

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
@kolchfa-aws kolchfa-aws added backport 1.3 PR: Backport label for v1.3.x backport 2.0 PR: Backport label for v2.0.x backport 2.1 PR: Backport label for 2.1 backport 2.2 PR: Backport label for 2.2 backport 2.3 PR: Backport label for 2.3 backport 2.4 PR: Backport label for 2.4 labels Dec 7, 2022
@kolchfa-aws kolchfa-aws merged commit 57c4683 into main Dec 7, 2022
@opensearch-trigger-bot
Copy link

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-2134-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 57c4683ce1e05747088fff980e4d6b3dae250fe3
# Push it to GitHub
git push --set-upstream origin backport/backport-2134-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-2134-to-1.3.

@opensearch-trigger-bot
Copy link

The backport to 2.0 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.0 2.0
# Navigate to the new working tree
cd .worktrees/backport-2.0
# Create a new branch
git switch --create backport/backport-2134-to-2.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 57c4683ce1e05747088fff980e4d6b3dae250fe3
# Push it to GitHub
git push --set-upstream origin backport/backport-2134-to-2.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.0

Then, create a pull request where the base branch is 2.0 and the compare/head branch is backport/backport-2134-to-2.0.

@opensearch-trigger-bot
Copy link

The backport to 2.1 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.1 2.1
# Navigate to the new working tree
cd .worktrees/backport-2.1
# Create a new branch
git switch --create backport/backport-2134-to-2.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 57c4683ce1e05747088fff980e4d6b3dae250fe3
# Push it to GitHub
git push --set-upstream origin backport/backport-2134-to-2.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.1

Then, create a pull request where the base branch is 2.1 and the compare/head branch is backport/backport-2134-to-2.1.

@opensearch-trigger-bot
Copy link

The backport to 2.2 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.2 2.2
# Navigate to the new working tree
cd .worktrees/backport-2.2
# Create a new branch
git switch --create backport/backport-2134-to-2.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 57c4683ce1e05747088fff980e4d6b3dae250fe3
# Push it to GitHub
git push --set-upstream origin backport/backport-2134-to-2.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.2

Then, create a pull request where the base branch is 2.2 and the compare/head branch is backport/backport-2134-to-2.2.

@opensearch-trigger-bot
Copy link

The backport to 2.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.3 2.3
# Navigate to the new working tree
cd .worktrees/backport-2.3
# Create a new branch
git switch --create backport/backport-2134-to-2.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 57c4683ce1e05747088fff980e4d6b3dae250fe3
# Push it to GitHub
git push --set-upstream origin backport/backport-2134-to-2.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.3

Then, create a pull request where the base branch is 2.3 and the compare/head branch is backport/backport-2134-to-2.3.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 7, 2022
* Adds geo-bounding box query documentation

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Removed geoshape

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Added geohash examples

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Incorporated doc review comments

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Implemented editorial feedback

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
(cherry picked from commit 57c4683)
Naarcha-AWS pushed a commit that referenced this pull request Dec 7, 2022
* Adds geo-bounding box query documentation

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Removed geoshape

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Added geohash examples

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Incorporated doc review comments

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

* Implemented editorial feedback

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>

Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
(cherry picked from commit 57c4683)

Co-authored-by: kolchfa-aws <105444904+kolchfa-aws@users.noreply.github.com>
@Naarcha-AWS Naarcha-AWS deleted the Fix1815-geo-bounding-box-query branch March 28, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.3 PR: Backport label for v1.3.x backport 2.0 PR: Backport label for v2.0.x backport 2.1 PR: Backport label for 2.1 backport 2.2 PR: Backport label for 2.2 backport 2.3 PR: Backport label for 2.3 backport 2.4 PR: Backport label for 2.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC]Add geo-bounding box query documentation
5 participants