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

geo/geogfn: implement ST_Segmentize({geography,float8}) #48368

Closed
otan opened this issue May 4, 2020 · 0 comments · Fixed by #49211
Closed

geo/geogfn: implement ST_Segmentize({geography,float8}) #48368

otan opened this issue May 4, 2020 · 0 comments · Fixed by #49211
Labels
A-geography-builtins Builtins which have geography as args. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@otan
Copy link
Contributor

otan commented May 4, 2020

Implement ST_Segmentize on arguments {geography,float8}, which should adopt PostGIS behaviour.

Observers: Please react to this issue if you need this functionality.

For Geography builtins, please do the following:

  • Ideally add a relevant helper function in pkg/geo/geogfn (parse and output related functions can go in pkg/geo). Add exhaustive unit tests here - you can run through example test cases and make sure that PostGIS and CRDB return the same result within a degree of accuracy (1cm for geography).
  • Create a new builtin that references this function in pkg/sql/sem/builtins/geo_builtins.go. Note that we currently do not support optional arguments, so we define functions that have optional arguments once without the optional argument (using the default value in the optional position), and once with the optional argument.
  • Modify the tests in pkg/sql/logictest/testdata/logic_test/geospatial to call this functionality at least once. You can call make testbaselogic FILES='geospatial' TESTFLAGS='-rewrite' to regenerate the output. Tests here should just ensure the builtin is linked end to end (your exhaustive unit tests go the above mentioned packages!).
  • Ensure the documentation is regenerated by calling make buildshort. You can also play with it by calling ./cockroach demo --empty afterwards.
  • Submit your PR - make sure to follow the guidelines from creating your first PR.

You can follow #48529 for an example PR.

The following additional guidance has been issued on implementing this function:

Applies to spheres only using the WGS84 Datum. S2's interpolate function should help here.

🤖 This issue was synced with a spreadsheet by gsheets-to-github-issues by otan on 2021-02-21T20:41:45Z. Changes to titles, body and labels may be overwritten.

@otan otan added A-spatial Spatial work that is *not* related to builtins. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) good first issue labels May 4, 2020
@otan otan changed the title geo/geogfn: implement st_segmentize {geography,float8} geo/geogfn: implement st_segmentize({geography,float8}) May 4, 2020
@otan otan added A-geospatial-builtin and removed A-spatial Spatial work that is *not* related to builtins. A-geospatial-builtin labels May 4, 2020
@otan otan changed the title geo/geogfn: implement st_segmentize({geography,float8}) geo/geogfn: implement ST_Segmentize({geography,float8}) May 4, 2020
@otan otan added A-geography-builtins Builtins which have geography as args. and removed A-geospatial-builtins labels May 4, 2020
@otan otan added E-easy Easy issue to tackle, requires little or no CockroachDB experience and removed good first issue labels May 13, 2020
abhishek20123g added a commit to abhishek20123g/cockroach that referenced this issue May 18, 2020
Fixes: cockroachdb#48368

This PR adds ST_Segmentize({geography,float8}) builtin function
which allows modify given geography such that no segment longer
than the given max_segment_length.

Release note (sql change): This PR implements ST_Segmentize
builtin functions.
@otan otan removed the E-easy Easy issue to tackle, requires little or no CockroachDB experience label May 19, 2020
abhishek20123g added a commit to abhishek20123g/cockroach that referenced this issue May 19, 2020
Fixes: cockroachdb#48368

This PR adds ST_Segmentize({geography,float8}) builtin function
which allows modify given geography such that no segment longer
than the given max_segment_length.

Release note (sql change): This PR implements ST_Segmentize
builtin function.
abhishek20123g added a commit to abhishek20123g/cockroach that referenced this issue May 19, 2020
Fixes: cockroachdb#48368

This PR adds ST_Segmentize({geography,float8}) builtin function
which allows modify given geography such that no segment longer
than the given max_segment_length.

Release note (sql change): This PR implements ST_Segmentize
builtin function.
abhishek20123g added a commit to abhishek20123g/cockroach that referenced this issue May 19, 2020
Fixes: cockroachdb#48368

This PR adds ST_Segmentize({geography,float8}) builtin function
which allows modify given geography such that no segment longer
than the given max_segment_length.

Release note (sql change): This PR implements ST_Segmentize
builtin function.
craig bot pushed a commit that referenced this issue May 21, 2020
49211: builtin: Implements ST_Segmentize builtin function r=otan a=abhishek20123g

Fixes: #48368

This PR adds ST_Segmentize({geography,float8}) builtin function
which allows modify given geography such that no segment longer
than the given max_segment_length.

Release note (sql change): This PR implements ST_Segmentize
builtin functions.

49270: cmd/reduce: use correct error detection; update license skipping r=mjibson a=mjibson

Error detection was incorrect and would panic, use a type instead of
value check.

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
@craig craig bot closed this as completed in 20eda8a May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-geography-builtins Builtins which have geography as args. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant