From: Sandro Santilli Date: Sat, 26 Dec 2015 12:23:54 +0000 (+0000) Subject: Fix crash splitting faces used by multiple TopoGeometry objects X-Git-Tag: 2.3.0beta1~313 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9b1b9056a9724384de095920cd9a64c58cef8593;p=postgis Fix crash splitting faces used by multiple TopoGeometry objects Thanks rulus for spotting and analyzing the bug. See #3407 git-svn-id: http://svn.osgeo.org/postgis/trunk@14522 b70326c6-7e19-0410-871a-916f4a2858ee --- diff --git a/topology/postgis_topology.c b/topology/postgis_topology.c index d7375dfd0..9a057f3aa 100644 --- a/topology/postgis_topology.c +++ b/topology/postgis_topology.c @@ -2028,53 +2028,62 @@ cb_updateTopoGeomFaceSplit ( const LWT_BE_TOPOLOGY* topo, } ntopogeoms = SPI_processed; - for ( i=0; ivals[i]; - TupleDesc tdesc = SPI_tuptable->tupdesc; - int negate; - int element_id; - int topogeo_id; - int layer_id; - int element_type; + resetStringInfo(sql); + appendStringInfo(sql, "INSERT INTO \"%s\".relation VALUES ", topo->name); + for ( i=0; ivals[i]; + TupleDesc tdesc = SPI_tuptable->tupdesc; + int negate; + int element_id; + int topogeo_id; + int layer_id; + int element_type; + + if ( ! getNotNullInt32( row, tdesc, 1, &element_id ) ) { + cberror(topo->be_data, + "unexpected null element_id in \"%s\".relation", + topo->name); + return 0; + } + negate = ( element_id < 0 ); - if ( ! getNotNullInt32( row, tdesc, 1, &element_id ) ) { - cberror(topo->be_data, - "unexpected null element_id in \"%s\".relation", - topo->name); - return 0; - } - negate = ( element_id < 0 ); + if ( ! getNotNullInt32( row, tdesc, 2, &topogeo_id ) ) { + cberror(topo->be_data, + "unexpected null topogeo_id in \"%s\".relation", + topo->name); + return 0; + } - if ( ! getNotNullInt32( row, tdesc, 2, &topogeo_id ) ) { - cberror(topo->be_data, - "unexpected null topogeo_id in \"%s\".relation", - topo->name); - return 0; - } + if ( ! getNotNullInt32( row, tdesc, 3, &layer_id ) ) { + cberror(topo->be_data, + "unexpected null layer_id in \"%s\".relation", + topo->name); + return 0; + } - if ( ! getNotNullInt32( row, tdesc, 3, &layer_id ) ) { - cberror(topo->be_data, - "unexpected null layer_id in \"%s\".relation", - topo->name); - return 0; - } + if ( ! getNotNullInt32( row, tdesc, 4, &element_type ) ) { + cberror(topo->be_data, + "unexpected null element_type in \"%s\".relation", + topo->name); + return 0; + } - if ( ! getNotNullInt32( row, tdesc, 4, &element_type ) ) { - cberror(topo->be_data, - "unexpected null element_type in \"%s\".relation", - topo->name); - return 0; - } + if ( i ) appendStringInfoChar(sql, ','); + appendStringInfo(sql, + "(%d,%d,%" LWTFMT_ELEMID ",%d)", + topogeo_id, layer_id, negate ? -new_face1 : new_face1, element_type); - resetStringInfo(sql); - appendStringInfo(sql, - "INSERT INTO \"%s\".relation VALUES (" - "%d,%d,%" LWTFMT_ELEMID ",%d)", topo->name, - topogeo_id, layer_id, negate ? -new_face1 : new_face1, element_type); + if ( new_face2 != -1 ) { + appendStringInfo(sql, + ",(%d,%d,%" LWTFMT_ELEMID ",%d)", + topogeo_id, layer_id, negate ? -new_face2 : new_face2, element_type); + } + } POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit query: %s", sql->data); - spi_result = SPI_execute(sql->data, false, 0); MemoryContextSwitchTo( oldcontext ); /* switch back */ if ( spi_result != SPI_OK_INSERT ) { @@ -2084,31 +2093,11 @@ cb_updateTopoGeomFaceSplit ( const LWT_BE_TOPOLOGY* topo, return 0; } if ( SPI_processed ) topo->be_data->data_changed = true; - if ( new_face2 != -1 ) { - resetStringInfo(sql); - appendStringInfo(sql, - "INSERT INTO \"%s\".relation VALUES (" - "%d,%d,%" LWTFMT_ELEMID ",%d)", topo->name, - topogeo_id, layer_id, negate ? -new_face2 : new_face2, element_type); - - POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit query: %s", sql->data); - - spi_result = SPI_execute(sql->data, false, 0); - MemoryContextSwitchTo( oldcontext ); /* switch back */ - if ( spi_result != SPI_OK_INSERT ) { - cberror(topo->be_data, "unexpected return (%d) from query execution: %s", - spi_result, sql->data); - pfree(sqldata.data); - return 0; - } - if ( SPI_processed ) topo->be_data->data_changed = true; - } } - /* TODO: release string info */ - POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit: updated %d topogeoms", ntopogeoms); + pfree(sqldata.data); return 1; } diff --git a/topology/test/regress/st_addedgemodface.sql b/topology/test/regress/st_addedgemodface.sql index 7c3518847..31caa4481 100644 --- a/topology/test/regress/st_addedgemodface.sql +++ b/topology/test/regress/st_addedgemodface.sql @@ -461,6 +461,20 @@ SELECT 'T28', 'E'||edge_id, next_left_edge, next_right_edge, UNION VALUES (4),(5) ) ORDER BY edge_id; +-- +-- Split a face referenced by multiple TopoGeometries +-- +-- See https://trac.osgeo.org/postgis/ticket/3407 +-- +INSERT INTO city_data.fp VALUES ('F17', + topology.CreateTopoGeom('city_data', 3, 1, '{{17,3}}')); +INSERT INTO newedge SELECT 29 as id, topology.st_addedgemodface('city_data', + 14, 16, 'LINESTRING(21 14, 9 22)'); +SELECT 'T29', 'E'||edge_id, next_left_edge, next_right_edge, + left_face, right_face FROM + city_data.edge WHERE edge_id IN (21, 33, 19, 34, + ( SELECT edge_id FROM newedge WHERE id = 29 ) ) + ORDER BY edge_id; --------------------------------------------------------------------- -- Check new relations and faces status @@ -489,4 +503,3 @@ SELECT 'F'||face_id, st_astext(mbr) FROM city_data.face ORDER BY face_id; DROP TABLE newedge; SELECT topology.DropTopology('city_data'); - diff --git a/topology/test/regress/st_addedgemodface_expected b/topology/test/regress/st_addedgemodface_expected index 69d18c448..31b944c13 100644 --- a/topology/test/regress/st_addedgemodface_expected +++ b/topology/test/regress/st_addedgemodface_expected @@ -162,7 +162,13 @@ T28|E44|-43|43|0|34 T28|E52|-53|4|32|24 T28|E53|-5|52|34|32 T28|E54|5|-54|34|33 -F3,F4|{3:3,3:4,3:10,3:16,3:17} +T29|E19|33|27|17|10 +T29|E21|6|34|0|35 +T29|E33|-55|-6|17|3 +T29|E34|55|9|35|16 +T29|E55|-21|19|35|17 +F17|{3:17,3:35} +F3,F4|{3:3,3:4,3:10,3:16,3:17,3:35} F5,N4|{1:4,3:5,3:11} F0| F1|POLYGON((3 30,3 38,16 38,16 30,3 30)) @@ -199,4 +205,5 @@ F31|POLYGON((19 31,19 38,26 38,26 31,19 31)) F32|POLYGON((36 33,36 38,57 38,57 33,36 33)) F33|POLYGON((38 40,38 43,41 43,41 40,38 40)) F34|POLYGON((35 25,35 45,63 45,63 25,35 25)) +F35|POLYGON((9 14,9 22,21 22,21 14,9 14)) Topology 'city_data' dropped diff --git a/topology/test/regress/st_addedgenewfaces.sql b/topology/test/regress/st_addedgenewfaces.sql index b3b296f1d..e86f7f9f2 100644 --- a/topology/test/regress/st_addedgenewfaces.sql +++ b/topology/test/regress/st_addedgenewfaces.sql @@ -461,6 +461,21 @@ SELECT 'T28', 'E'||edge_id, next_left_edge, next_right_edge, UNION VALUES (4),(5) ) ORDER BY edge_id; +-- +-- Split a face referenced by multiple TopoGeometries +-- +-- See https://trac.osgeo.org/postgis/ticket/3407 +-- +INSERT INTO city_data.fp VALUES ('F25', + topology.CreateTopoGeom('city_data', 3, 1, '{{25,3}}')); +INSERT INTO newedge SELECT 29 as id, topology.st_addedgemodface('city_data', + 14, 16, 'LINESTRING(21 14, 9 22)'); +SELECT 'T29', 'E'||edge_id, next_left_edge, next_right_edge, + left_face, right_face FROM + city_data.edge WHERE edge_id IN (21, 33, 19, 34, + ( SELECT edge_id FROM newedge WHERE id = 29 ) ) + ORDER BY edge_id; + --------------------------------------------------------------------- -- Check new relations and faces status --------------------------------------------------------------------- diff --git a/topology/test/regress/st_addedgenewfaces_expected b/topology/test/regress/st_addedgenewfaces_expected index 8fa933c27..f98b2290d 100644 --- a/topology/test/regress/st_addedgenewfaces_expected +++ b/topology/test/regress/st_addedgenewfaces_expected @@ -162,7 +162,13 @@ T28|E44|-43|43|0|50 T28|E52|-53|4|47|45 T28|E53|-5|52|50|47 T28|E54|5|-54|50|49 -F3,F4|{3:10,3:11,3:22,3:24,3:25} +T29|E19|33|27|25|11 +T29|E21|6|34|0|51 +T29|E33|-55|-6|25|22 +T29|E34|55|9|51|24 +T29|E55|-21|19|51|25 +F25|{3:25,3:51} +F3,F4|{3:10,3:11,3:22,3:24,3:25,3:51} F5,N4|{1:4,3:12,3:13} F0| F1|POLYGON((3 30,3 38,16 38,16 30,3 30)) @@ -199,4 +205,5 @@ F45|POLYGON((36 28,36 38,57 38,57 28,36 28)) F47|POLYGON((36 33,36 38,57 38,57 33,36 33)) F49|POLYGON((38 40,38 43,41 43,41 40,38 40)) F50|POLYGON((35 25,35 45,63 45,63 25,35 25)) +F51|POLYGON((9 14,9 22,21 22,21 14,9 14)) Topology 'city_data' dropped