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

geomfn: st_simplify with NaN causes node to crash #63620

Closed
rafiss opened this issue Apr 14, 2021 · 3 comments · Fixed by #63760
Closed

geomfn: st_simplify with NaN causes node to crash #63620

rafiss opened this issue Apr 14, 2021 · 3 comments · Fixed by #63760
Labels
A-spatial Spatial work that is *not* related to builtins. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. good first issue

Comments

@rafiss
Copy link
Collaborator

rafiss commented Apr 14, 2021

This query causes the node to crash without leaving any logs behind.

SELECT st_simplify('0104000000020000000101000000AC885A457886F9C1C9EEF30F7308F3C101010000007411E55B3F66EDC10BBEFA0ABF45F0C1':::GEOMETRY, 'nan':::FLOAT8);

It works on PostGIS though

postgres@localhost:postgres> SELECT st_simplify('0104000000020000000101000000AC885A457886F9C1C9EEF30F7308F3C101010000007411E55B3F66EDC10BBEFA0ABF45F0C1'::GEOMETRY, 'NaN'::FLOAT8)
+--------------------------------------------------------------------------------------------------------+
| st_simplify                                                                                            |
|--------------------------------------------------------------------------------------------------------|
| 0104000000020000000101000000AC885A457886F9C1C9EEF30F7308F3C101010000007411E55B3F66EDC10BBEFA0ABF45F0C1 |
+--------------------------------------------------------------------------------------------------------+
SELECT 1

postgres@localhost:postgres> SELECT st_astext(st_simplify('0104000000020000000101000000AC885A457886F9C1C9EEF30F7308F3C101010000007411E55B3F66EDC10BBEFA0ABF45F0C1'::GEOMETRY,
 'NaN'::FLOAT8))
+-----------------------------------------------------------------------------------+
| st_astext                                                                         |
|-----------------------------------------------------------------------------------|
| MULTIPOINT(-6851888213.65837 -5109133567.2458,-3945921247.15838 -4368101551.6714) |
+-----------------------------------------------------------------------------------+
SELECT 1

cc @otan @andyyang890

@rafiss rafiss added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-spatial Spatial work that is *not* related to builtins. labels Apr 14, 2021
@rafiss rafiss changed the title geomfn: st_simplify causes node to crash geomfn: st_simplify with NaN causes node to crash Apr 14, 2021
@otan
Copy link
Contributor

otan commented Apr 14, 2021

it works in PostGIS because they a) special case points to return themselves and b) have their own algorithm for simplifying from what i can tell.

we're relying on GEOS instead which does not deliver the goods.

We should make the following changes:
a) Simplify with points/multipoints should return themselves (use ShapeType2D() to determine shape type)
b) Simplify with NaN should return themselves.

This can be done before the check to GEOS here:

func Simplify(g geo.Geometry, tolerance float64) (geo.Geometry, error) {
simplifiedEWKB, err := geos.Simplify(g.EWKB(), tolerance)
if err != nil {
return geo.Geometry{}, err

With tests added here:

func TestSimplify(t *testing.T) {
testCases := []struct {
wkt string
tolerance float64
expected string
}{
{

@techytoes
Copy link
Contributor

Hey @rafiss, can I pick this up?

@rafiss
Copy link
Collaborator Author

rafiss commented Apr 14, 2021

@techytoes sure, please do! @otan's directions above should let you work on this but let me know if you are stuck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spatial Spatial work that is *not* related to builtins. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants