Opened 12 years ago
Closed 12 years ago
#1998 closed defect (fixed)
Crash of topogeo_AddPolygon() with "ERROR: query string argument of EXECUTE is null"
Reported by: | wimned | Owned by: | strk |
---|---|---|---|
Priority: | blocker | Milestone: | PostGIS 2.0.2 |
Component: | topology | Version: | master |
Keywords: | topogeo_AddPolygon, history | Cc: |
Description
Complete message:
ERROR: query string argument of EXECUTE is null CONTEXT: PL/pgSQL function "st_modedgesplit" line 106 at EXECUTE statement PL/pgSQL function "topogeo_addpoint" line 65 at assignment PL/pgSQL function "topogeo_addlinestring" line 92 at assignment SQL statement "SELECT array_cat(edges, array_agg(x)) FROM ( select topology.TopoGeo_addLinestring(atopology, rec.geom, tol) as x ) as foo"
Version:
POSTGIS="2.1.0SVN r10195" GEOS="3.3.4-CAPI-1.7.3" PROJ="Rel. 4.7.1, 23 September 2009" LIBXML="2.7.8" TOPOLOGY
After pol1 is added, node 2, the begin and end of the loop of pol1 is removed through ST_ModEdgeHeal(). The code breaks when pol2 is added after this. Rearranging the geometry of pol1, so the common point of pol0 and pol1 is the begin/endpoint of the loop of pol1, prevents the crash.
The code:
CREATE OR REPLACE FUNCTION testerror1() returns void as $$ declare pol0 geometry; declare pol1 geometry; declare pol2 geometry; declare node_rec RECORD; begin raise notice 'version: %', postgis_full_version(); perform CreateTopology('wimpy', 4326, 0.0000001); CREATE VIEW face_geom AS select i.face_id, ST_GetFaceGeometry('wimpy', i.face_id) as geom from wimpy.face as i where face_id != 0; pol0 = ST_GeometryFromText( 'POLYGON((76.5282669067383 9.43292331695557,76.5287322998047 9.43261814117432,76.5285415649414 9.43255615234375,76.5282669067383 9.43292331695557))', 4326); pol1 = ST_GeometryFromText( 'POLYGON((76.5267181396484 9.4406852722168,76.5282669067383 9.43292331695557, 76.5264434814453 9.43406867980957, 76.5267181396484 9.4406852722168))',4326); pol2 = ST_GeometryFromText( 'POLYGON((76.524 9.437, 76.5267181396484 9.4406852722168,76.5264434814453 9.43406867980957,76.524 9.437))', 4326); perform topogeo_AddPolygon('wimpy', pol0); perform topogeo_AddPolygon('wimpy', pol1); perform cleanse_nodes(); perform topogeo_AddPolygon('wimpy', pol2); END $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION cleanse_nodes() returns void AS $$ DECLARE node_rec RECORD; BEGIN for node_rec in select node_id from node loop perform cleanse_node(node_rec.node_id); end loop; END $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION cleanse_node(passed_node_id integer) returns void AS $$ DECLARE edge_ids integer[]; BEGIN --raise notice 'check node %', passed_node_id; select array_agg(abs(t.edge)) into edge_ids from GetNodeEdges('wimpy', passed_node_id) as t(seq,edge); if array_length(edge_ids,1) = 2 and edge_ids[1] != edge_ids[2] then raise notice 'remove node %', passed_node_id; perform ST_ModEdgeHeal('wimpy', edge_ids[1], edge_ids[2]); end if; END $$ LANGUAGE plpgsql;
Attachments (1)
Change History (17)
by , 12 years ago
follow-up: 3 comment:1 by , 12 years ago
Component: | postgis → topology |
---|---|
Milestone: | PostGIS 2.1.0 → PostGIS 2.0.2 |
Owner: | changed from | to
Note that the test script doesn't work on PostgreSQL 9.1.5. It's a search_path issue, your cleanse_node function references a "node" table but maybe should reference a "wimpy.node" one
comment:3 by , 12 years ago
Replying to strk:
Note that the test script doesn't work on PostgreSQL 9.1.5. It's a search_path issue, your cleanse_node function references a "node" table but maybe should reference a "wimpy.node" one
I call "export PGOPTIONS="-c SEARCH_PATH=wimpy,topology,public"" in the calling bash script.
comment:4 by , 12 years ago
The topology is made invalid by cleanse_node:
=# select * from validatetopology('wimpy'); error | id1 | id2 ------------------------------------+-----+----- edge crosses edge | 1 | 2 edge start node geometry mis-match | 2 | 1 edge end node geometry mis-match | 2 | 1 (3 rows)
Before the cleansing you have 3 edges and 2 nodes in a valid topology.
comment:5 by , 12 years ago
So basically the bug is in ST_ModEdgeHeal, which lets you break the topology by picking the wrong node to heal at. Need a smaller testcase next, to put with the rest of tests in the suite.
comment:6 by , 12 years ago
The problem is with the assumption that ST_LineMerge will do the right thing, which of course can't be true. So the node is choosed correctly, but the line merging is done in the wrong way.
comment:7 by , 12 years ago
smaller testcase:
CREATE OR REPLACE FUNCTION testerror1() returns void as $$ declare pol0 geometry; declare pol1 geometry; begin raise notice 'version: %', postgis_full_version(); perform CreateTopology('wimpy', 4326, 0.0000001); pol0 = ST_GeometryFromText( 'POLYGON((1 1,1 2,2 2,2 1,1 1))', 4326); pol1 = ST_GeometryFromText( 'POLYGON((0 0,0 1,1 1,1 0,0 0))', 4326); perform topogeo_AddPolygon('wimpy', pol0); perform topogeo_AddPolygon('wimpy', pol1); perform cleanse_nodes(); END $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION cleanse_nodes() returns void AS $$ DECLARE node_rec RECORD; BEGIN for node_rec in select node_id from wimpy.node loop perform cleanse_node(node_rec.node_id); end loop; END $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION cleanse_node(passed_node_id integer) returns void AS $$ DECLARE edge_ids integer[]; BEGIN --raise notice 'check node %', passed_node_id; select array_agg(abs(t.edge)) into edge_ids from GetNodeEdges('wimpy', passed_node_id) as t(seq,edge); if array_length(edge_ids,1) = 2 and edge_ids[1] != edge_ids[2] then raise notice 'remove node %', passed_node_id; perform ST_ModEdgeHeal('wimpy', edge_ids[1], edge_ids[2]); end if; END $$ LANGUAGE plpgsql;
from validate_topology:
error | id1 | id2 ------------------------------------+-----+----- edge crosses edge | 1 | 2 edge start node geometry mis-match | 2 | 1 edge end node geometry mis-match | 2 | 1 (3 rows)
edges:
edge_id | st_astext ---------+--------------------------------- 2 | LINESTRING(0 0,1 0,1 1,0 1,0 0) 1 | LINESTRING(1 1,1 2,2 2,2 1,1 1) (2 rows)
nodes:
node_id | st_astext ---------+------------ 1 | POINT(1 1) (1 row)
looks a bit similar to ticket #1955
comment:8 by , 12 years ago
Created a workaround: After call of ST_ModEdgeHeal() the first point of geometry of the new edge is checked against the geometry of the start node. When not, points are shifted in the edge geometry until the first point matches the start node. The edge is removed using ST_RemEdgeModFace(). The fixed geometry is added using topogeo_AddLineString()
The code (probably could be made less clumsy…):
CREATE OR REPLACE FUNCTION cleanse_node(passed_node_id integer) returns void AS $$ DECLARE edge_ids integer[]; DECLARE start_node_id integer; DECLARE node_geom geometry; DECLARE edge_geom geometry; DECLARE edge_sz integer; BEGIN --raise notice 'check node %', passed_node_id; select array_agg(abs(t.edge)) into edge_ids from GetNodeEdges('wimpy', passed_node_id) as t(seq,edge); if array_length(edge_ids,1) = 2 and edge_ids[1] != edge_ids[2] then raise notice 'remove node %', passed_node_id; perform ST_ModEdgeHeal('wimpy', edge_ids[1], edge_ids[2]); select e.start_node into start_node_id from wimpy.edge_data as e where e.edge_id = edge_ids[1]; select e.geom into edge_geom from wimpy.edge_data as e where e.edge_id = edge_ids[1]; select n.geom into node_geom from wimpy.node as n where n.node_id = start_node_id; if not ST_Equals(node_geom, ST_PointN(edge_geom,1)) then raise notice 'edge % needs fixing at %', edge_ids[1], ST_ASText(node_geom); edge_geom = ST_RemovePoint(edge_geom, 0); edge_sz = ST_NPoints(edge_geom); for i in 1..edge_sz loop exit when ST_Equals(node_geom, ST_PointN(edge_geom,1)); edge_geom = ST_AddPoint(edge_geom, ST_PointN(edge_geom,1), -1); edge_geom = ST_RemovePoint(edge_geom, 0); end loop; edge_geom = ST_AddPoint(edge_geom, node_geom, -1); perform ST_RemEdgeModFace('wimpy', edge_ids[1]); perform topogeo_AddLineString('wimpy', edge_geom); end if; end if; END $$
comment:9 by , 12 years ago
Question: is this ticket going to be assigned? I'm not in a hurry though, my workaround just works fine.
comment:10 by , 12 years ago
Status: | new → assigned |
---|
Yep, consider it assigned to me. Not sure when I'll find the time to fix it though. Surely needs to be fixed before 2.0.2 is out.
comment:11 by , 12 years ago
strk - is this still an issue — are you okay with pushing this or are you going to get your butt in gear
comment:13 by , 12 years ago
Priority: | medium → blocker |
---|
comment:14 by , 12 years ago
Further simplification:
SELECT CreateTopology('t1998'); SELECT ST_AddIsoNode('t1998', 0, 'POINT(1 1)'); SELECT ST_AddIsoNode('t1998', 0, 'POINT(0 0)'); SELECT ST_AddEdgeModFace('t1998', 1, 1, 'LINESTRING(1 1,1 2,2 2,2 1,1 1)'); SELECT ST_AddEdgeModFace('t1998', 2, 1, 'LINESTRING(0 0,0 1,1 1)'); SELECT ST_AddEdgeModFace('t1998', 1, 2, 'LINESTRING(1 1,1 0,0 0)'); SELECT 'V1', ValidateTopology('t1998'); -- ok SELECT ST_ModEdgeHeal('t1998', 2, 3); SELECT 'V2', ValidateTopology('t1998'); -- mess
comment:15 by , 12 years ago
Ok so the curious effect of this case is that you start from the upper node, check which edges are sharing that node and request healing them. To the possible user surprise the edges _are_ healed, but the removed node is on the other side. I think the invariant we want to keep here is the direction of the first edge passed to the ModEdge function, but we of course have to give up the invariant that the start node of the first edge will remain the start node of the resulting "merged" edge (because in this case the first edge passed to the function will end up being the _second_ portion of the merged edge).
comment:16 by , 12 years ago
Keywords: | history added |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
the polygons