[postgis-tickets] r14522 - Fix crash splitting faces used by multiple TopoGeometry objects

Sandro Santilli strk at keybit.net
Sat Dec 26 04:23:54 PST 2015


Author: strk
Date: 2015-12-26 04:23:54 -0800 (Sat, 26 Dec 2015)
New Revision: 14522

Modified:
   trunk/topology/postgis_topology.c
   trunk/topology/test/regress/st_addedgemodface.sql
   trunk/topology/test/regress/st_addedgemodface_expected
   trunk/topology/test/regress/st_addedgenewfaces.sql
   trunk/topology/test/regress/st_addedgenewfaces_expected
Log:
Fix crash splitting faces used by multiple TopoGeometry objects

Thanks rulus for spotting and analyzing the bug.
See #3407

Modified: trunk/topology/postgis_topology.c
===================================================================
--- trunk/topology/postgis_topology.c	2015-12-26 09:53:47 UTC (rev 14521)
+++ trunk/topology/postgis_topology.c	2015-12-26 12:23:54 UTC (rev 14522)
@@ -2028,53 +2028,62 @@
   }
 
   ntopogeoms = SPI_processed;
-  for ( i=0; i<ntopogeoms; ++i )
+  if ( ntopogeoms )
   {
-    HeapTuple row = SPI_tuptable->vals[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; i<ntopogeoms; ++i )
+    {
+      HeapTuple row = SPI_tuptable->vals[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;
+      }
 
-    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 ( i ) appendStringInfoChar(sql, ',');
+      appendStringInfo(sql,
+        "(%d,%d,%" LWTFMT_ELEMID ",%d)",
+        topogeo_id, layer_id, negate ? -new_face1 : new_face1, element_type);
 
-    POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit query: %s", sql->data);
+      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 @@
       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;
 }
 

Modified: trunk/topology/test/regress/st_addedgemodface.sql
===================================================================
--- trunk/topology/test/regress/st_addedgemodface.sql	2015-12-26 09:53:47 UTC (rev 14521)
+++ trunk/topology/test/regress/st_addedgemodface.sql	2015-12-26 12:23:54 UTC (rev 14522)
@@ -461,6 +461,20 @@
     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 @@
 
 DROP TABLE newedge;
 SELECT topology.DropTopology('city_data');
-

Modified: trunk/topology/test/regress/st_addedgemodface_expected
===================================================================
--- trunk/topology/test/regress/st_addedgemodface_expected	2015-12-26 09:53:47 UTC (rev 14521)
+++ trunk/topology/test/regress/st_addedgemodface_expected	2015-12-26 12:23:54 UTC (rev 14522)
@@ -162,7 +162,13 @@
 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 @@
 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

Modified: trunk/topology/test/regress/st_addedgenewfaces.sql
===================================================================
--- trunk/topology/test/regress/st_addedgenewfaces.sql	2015-12-26 09:53:47 UTC (rev 14521)
+++ trunk/topology/test/regress/st_addedgenewfaces.sql	2015-12-26 12:23:54 UTC (rev 14522)
@@ -461,6 +461,21 @@
     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
 ---------------------------------------------------------------------

Modified: trunk/topology/test/regress/st_addedgenewfaces_expected
===================================================================
--- trunk/topology/test/regress/st_addedgenewfaces_expected	2015-12-26 09:53:47 UTC (rev 14521)
+++ trunk/topology/test/regress/st_addedgenewfaces_expected	2015-12-26 12:23:54 UTC (rev 14522)
@@ -162,7 +162,13 @@
 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 @@
 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



More information about the postgis-tickets mailing list