Cockroach: geo/geomfn: implement ST_SharedPaths({geometry,geometry})

[otan]

Implement ST_SharedPaths on arguments {geometry,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.

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

Implemented by GEOS.

_{:robot: This issue was synced with a spreadsheet by gsheets-to-github-issues by otan on 2020-11-15T23:41:27Z. Changes to titles, body and labels may be overwritten.}

[devenbhooshan]

what about this one @otan ?

[otan]

Go for it.

On Tue, 18 Aug 2020, 10:23 pm Deven Bhooshan, [e-mail]
wrote:

what about this one @otan https://github.com/otan ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/cockroachdb/cockroach/issues/49033#issuecomment-675858269,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AA32FQ5HUIX27OKVSJFYEB3SBNOVBANCNFSM4NAZPMEA
.

[devenbhooshan]

hey @otan

this is what I have understood so far

• geos.EnsureInit is initializing the geos

• I found GEOSSharedPaths_r in the C-Wrapper

• the implementation of sharedPath is under SharedPathsOp.cpp.

So this is what I think I will have to do.

• declare a cpp wrapper for GEOSSharedPaths_r under pkg/geo/geos/geos.h
• create a function in cockroach/pkg/geo/geos/geos.go and call the cpp function from this function
• call the function created in step 2 from cockroach/pkg/geo/geomfn

please, cmiiw. wanted to have a discussion before starting the implementation.

[otan]

sounds just about right, i have attached a PR in the description showing this process: https://github.com/cockroachdb/cockroach/pull/48552

As long as you roughly follow it, you should be sweet.

[devenbhooshan]

draft PR : https://github.com/cockroachdb/cockroach/pull/53307

[devenbhooshan]

I don't seem to find any valid representation for GeometryCollection currently implemented. I will have to implement DGeometryCollection just like DBox2D. @otan what do you suggest?

[otan]

GeometryCollection is a shape that can be represented by DGeometry / DGeography.
You should be able to use the same EWKB represented by those objects with GEOS.

[devenbhooshan]

let's close this?

[otan]

Ah next time type "resolves #prnum" in the PR description and Github will close it automatically!

Thanks for all the following up you have done on this, we really appreciate it!