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

feat: add some s2 geo functions #4823

Merged
merged 7 commits into from
Oct 15, 2024
Merged

Conversation

sunng87
Copy link
Member

@sunng87 sunng87 commented Oct 11, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This patch introduces some new s2 geo encoding functions.

More information: http://s2geometry.io/

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.

@sunng87 sunng87 requested a review from a team as a code owner October 11, 2024 21:08
@github-actions github-actions bot added the docs-required This change requires docs update. label Oct 11, 2024
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

Attention: Patch coverage is 47.42857% with 92 lines in your changes missing coverage. Please review.

Project coverage is 84.04%. Comparing base (aaa9b32) to head (e145d35).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4823      +/-   ##
==========================================
- Coverage   84.37%   84.04%   -0.33%     
==========================================
  Files        1127     1125       -2     
  Lines      205164   205140      -24     
==========================================
- Hits       173104   172416     -688     
- Misses      32060    32724     +664     

Copy link
Collaborator

@fengjiachun fengjiachun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@v0y4g3r v0y4g3r left a comment

Choose a reason for hiding this comment

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

Rest LGTM

src/common/function/src/scalars/geo/s2.rs Outdated Show resolved Hide resolved
Co-authored-by: Lei, HUANG <6406592+v0y4g3r@users.noreply.github.com>
src/common/function/src/scalars/geo/s2.rs Outdated Show resolved Hide resolved
src/common/function/src/scalars/geo/s2.rs Outdated Show resolved Hide resolved
src/common/function/src/scalars/geo/s2.rs Outdated Show resolved Hide resolved
src/common/function/src/scalars/geo/s2.rs Outdated Show resolved Hide resolved
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@waynexia waynexia enabled auto-merge October 15, 2024 06:30
@waynexia waynexia added this pull request to the merge queue Oct 15, 2024
Merged via the queue into GreptimeTeam:main with commit bb8b54b Oct 15, 2024
33 checks passed
@adminpass
Copy link

adminpass commented Oct 18, 2024

Please support. Thank you

ST_Contains
ST_Within
ST_Intersects

ST_Area
ST_Distance
ST_DistanceSphere

@sunng87
Copy link
Member Author

sunng87 commented Oct 18, 2024

@adminpass Thank you for letting me know! I wonder if you have a use-case with geospatial functions in GreptimeDB or other time-series database? It will be helpful if you can describe the scenario so we can prioritize.

@adminpass
Copy link

@adminpass Thank you for letting me know! I wonder if you have a use-case with geospatial functions in GreptimeDB or other time-series database? It will be helpful if you can describe the scenario so we can prioritize.

For example:

  1. Query coordinate data within the region
  2. Query data with a specified coordinate radius of 10km

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-required This change requires docs update.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants