Opened 12 years ago

Closed 6 years ago

#1855 closed defect (fixed)

'SQL/MM Spatial exception - curve not simple' exception at adding two polygons

Reported by: wimned Owned by: strk
Priority: medium Milestone: PostGIS 2.5.0
Component: topology Version: 2.0.x
Keywords: Cc:


The code:

 returns void AS
    declare geom0 geometry;
    declare geom1 geometry;
    perform CreateTopology('wimpy', 4326, 0.00001);

    geom0 = ST_GeometryFromText(
        'POLYGON((76.26727 9.85751,76.29001 9.90026,
                76.29942 9.86257,76.26727 9.85751))' , 4326);
    geom1 = ST_GeometryFromText(
        'POLYGON((76.31988 9.89696,76.30482 9.88391,
                76.2941 9.88391,76.29409 9.88391,76.29409 9.88392,
                76.29001 9.90026,76.31988 9.89696))' , 4326);
    perform topogeo_AddPolygon('wimpy', geom0);
    perform topogeo_AddPolygon('wimpy', geom1);
LANGUAGE plpgsql;

Raises the exception mentioned in the title.

The curve is:

LINESTRING(76.2940920774741 9.88391,76.29409 9.88391,76.2940920774741 9.88391)

Probable cause is a small triangular shaped overlap of the two polygons, about the size of the tolerance. Snapping leads to an edge of 3 points, begin and end point being the same.

I'm using a nightly build from launchpad on my ubuntu box:

deb precise main

I managed to edit a workaround in /usr/share/postgresql/9.1/contrib/postgis-2.1/topology.sql:

In function ST_AddEdgeModFace() at the test of the curve being simple

  IF NOT ST_IsSimple(acurve) THEN
    IF ST_NPoints(acurve) = 3 AND   ST_Equals(ST_PointN(acurve,1),ST_PointN(acurve,1))
	return 0; -- skip spike
    END IF;
    RAISE EXCEPTION 'SQL/MM Spatial exception - curve not simple %', ST_AsText(acurve);

Inspecting the code calling ST_AddEdgeModFace the return value must be checked, e.g. in TopoGeo_addLineString():

      id := topology.ST_AddEdgeModFace(atopology,start_node, end_node,
    END IF;

    if id != 0
    	RETURN NEXT id; --value 0 means invalid curve to be skipped
    end if;

These code changes result in the polygons to be entered into the topology properly. (though there is a small face between the two polygons).

The change doesn't break much, the conversion would have been stopped anyhow by the exception

Change History (24)

comment:1 by strk, 12 years ago

Those functions are expected to split whatever you pass in input and always succeed. An exception is a bug. Robustness related, most likely. Changes are this was already fixed with GEOS-3.3.4. Can you show your postgis_full_version() ?

comment:2 by wimned, 12 years ago

output is:

NOTICE:  version: POSTGIS="2.1.0SVN" GEOS="3.3.3-CAPI-1.7.4" PROJ="Rel. 4.7.1, 23 September 2009" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.1.0SVN" need upgrade) TOPOLOGY (topology procs from "2.1.0SVN" need upgrade)

So I'll wait for geos 3.3.4

comment:3 by strk, 12 years ago

GEOS-3.3.4 is out already, nothing to wait for, just go get it. Also, note that "needs upgrade" messages. They want you to load postgis_upgrade_20_minor.sql and topology_upgrade_20_minor.sql

Let me know if it works after all the upgrades.

comment:4 by strk, 12 years ago

Status: newassigned

Ok, I just tested your functions and could reproduce the problem. This is with POSTGIS="2.0.1SVN r9850" GEOS="3.4.0dev-CAPI-1.8.0 r3670"

comment:5 by strk, 12 years ago

Line 954 of topology/sql/populate.sql contains a comment about code that seems to be there specifically to handle this case, just evidently in the wrong way (ST_MakeValid doesn't clean non-simple lines)

comment:6 by strk, 12 years ago

That line was there to fix #1650. In that case the snapping made the line collapse to a point. In this one it's still a line but non-simple. Will need to add the new testcase into topology/test/regress/topogeo_addlinestring.sql and then fix…

comment:7 by strk, 12 years ago

Extremely simplified case exposing the bug:

SELECT CreateTopology('wimpy');
SELECT topogeo_AddLinestring('wimpy', 'LINESTRING(0 0, 10 0, 0 1)', 2);

comment:8 by strk, 12 years ago

so, after fixing the simplicity of the line next problem is that this cleanup operation changes first and last point, so that you can't call ST_AddEdgeModFace relying on the previously added start/endpoints. In the above scenario the cleanup to (0 0, 10 0) will introduce a new node (10 0) which wasn't known before cleaning up. Sounds like a possibly iterative process in that we could still add (10 0) as the new endpoint but then would need to re-snap possibly introducing more troubles.

As seen in many other cases, a predictable snapping strategy would greatly help all of this.

comment:9 by strk, 12 years ago

Another example simpler to understand, and harder to fix:

SELECT CreateTopology('t');
SELECT topogeo_AddLinestring('t', 'LINESTRING(0 0, 0 100)');
SELECT topogeo_AddLinestring('t', 'LINESTRING(10 51, -100 50, 10 49)', 2);

The intersection points are closer than the tolerance. The code fails at consistently merging them into a single node.

comment:10 by strk, 12 years ago

Milestone: PostGIS 2.0.1PostGIS 2.0.2

won't be working on this in time for 2.0.1 (and not even sure it'll ever work in the 2.0 branch)

comment:11 by strk, 11 years ago

Milestone: PostGIS 2.0.2PostGIS 2.0.3

comment:12 by strk, 11 years ago

Milestone: PostGIS 2.0.4PostGIS 2.1.0

comment:13 by robe, 11 years ago

strk I assume this is still an issue? I'd like to get to zero tickets soon since the next release I do will be an rc. If this isn't fixed and you don't plan to in next 2 weeks please push to 2.2

comment:14 by strk, 11 years ago

We need a 2.1.1 milestone,don't we ? There is a 2.0.4 one. Or we could add a 2.1.x milestone.

comment:15 by robe, 11 years ago

Milestone: PostGIS 2.1.0PostGIS 2.1.1

I was going to wait til 2.1.0 was out, but guess there is no hurt in palnning a tincy bit ahead since are doing that with 2.2.0 anyway.

comment:16 by robe, 10 years ago

Milestone: PostGIS 2.1.1PostGIS 2.2.0

comment:17 by strk, 8 years ago

I'm looking at this case. It seems that code exist which is meant to handle this case (of an edge self-collapsing due to snapping) but it's evidently uneffective. The problem is that it calls ST_MakeValid which only does something if the input is invalid, which is never the case for linestrings. We need code that fixes non-simplicity instead

comment:18 by strk, 8 years ago

NOTE: the ST_MakeValid is not completely useless for lines but handles the collapse-to-singlepoint case, which is still needed.

comment:19 by strk, 8 years ago

Milestone: PostGIS 2.2.0PostGIS 2.3.0

comment:20 by robe, 7 years ago

Milestone: PostGIS 2.3.0PostGIS 2.4.0

comment:21 by dbaston, 7 years ago

Milestone: PostGIS 2.4.0PostGIS 2.5.0

comment:22 by strk, 6 years ago

Both simplified cases are fixed with the PR suggested in #3838

comment:23 by strk, 6 years ago

The original case is also fixed. As of 0911624e7570fb026cea56772ac746e64bf3a457 from the merge request

comment:24 by strk, 6 years ago

Resolution: fixed
Status: assignedclosed

In 16038:

Add tests for topology bug #1855

Previous commits fixes #1855 too

Note: See TracTickets for help on using tickets.