geo/geomfn: implement ST_FlipCoordinates({geometry})

[otan]

Implement ST_FlipCoordinates on arguments {geometry}, which should adopt PostGIS behaviour.

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

For Geometry builtins, please do the following:

• Ideally add a relevant helper function in pkg/geo/geomfn (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).
  • When using GEOS, you can reference the C API for which functions are available. Unfortunately, Windows is not currently supported when using GEOS.
• 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 #48552 for an example PR.

_{🤖 This issue was synced with a spreadsheet by gsheets-to-github-issues by otan on 2023-09-03T23:16:38Z. Changes to titles, body and labels may be overwritten.}

[themistoklik]

Can I work on this?

[otan]

Yes

…

On Sat, 22 Aug 2020, 9:16 am Themis, ***@***.***> wrote: Can I work on this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#48932 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA32FQ473YN6YQWSPCONBOLSB7VMZANCNFSM4NAZN3RQ> .

[themistoklik]

Used reverse as a guide to compensate for no Go/GIS experience.

Linters + Local unit/logic tests pass but getting timeouts on the big ones that ran with make check or make pre-push so I'm not gonna open a PR just yet, since I got no idea why the fails occur.

1. My change is here temporarily. I wanted to check if you think I should PR nonetheless or you wanted to take a quick scan at my change first.
2. PostGIS doc mentions support for M/Z indices and various surfaces that I do not immediately see supported in the codebase. In the unit test for reverse I only saw 2d geometries being tested, so I went ahead and did the same for my command. Combine that with this, I thought 2d is supported only, still wanted the assumption validated. Is the 2d testing only OK?

[otan]

I don't think those commands are expected to not time out. The issue description has instructions on a small subset of test work - otherwise feel free to senda PR and let CI do the work! You can test 3D/4D with geom.T objects. We don't support them right now. If yours doesn't test for it don't worry.

…

On Sun, 23 Aug 2020, 1:52 pm Themis, ***@***.***> wrote: Used reverse <https://github.com/cockroachdb/cockroach/blob/master/pkg/geo/geomfn/reverse.go> as a guide to compensate for no Go/GIS experience. Linters + Local unit/logic tests pass but getting timeouts on the big ones that ran with make check or make pre-push so I'm not gonna open a PR just yet, since I got no idea why the fails occur. 1. My change is here <themistoklik@f61c7fc> temporarily. I wanted to check if you think I should PR nonetheless or you wanted to take a quick scan at my change first. 2. PostGIS doc <https://postgis.net/docs/ST_FlipCoordinates.html> mentions support for M/Z indices and various surfaces that I do not immediately see supported in the codebase. In the unit test <https://github.com/cockroachdb/cockroach/blob/master/pkg/geo/geomfn/reverse_test.go> for reverse I only saw 2d geometries being tested, so I went ahead and did the same for my command. Is this ok? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#48932 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA32FQ76FMXLGPKTWAP76SDSCF6PXANCNFSM4NAZN3RQ> .