[SCM] PostGIS branch master updated. 3.5.0alpha2-17-g80806376a

git at osgeo.org git at osgeo.org
Fri Jul 26 10:43:26 PDT 2024


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "PostGIS".

The branch, master has been updated
       via  80806376a566ebd1b2edfe0dd0a69dc1c3f718cd (commit)
      from  87a34bf2fe13c48c3fd6c026bdb7989c66d5e162 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 80806376a566ebd1b2edfe0dd0a69dc1c3f718cd
Author: Sandro Santilli <strk at kbt.io>
Date:   Fri Jul 26 17:26:40 2024 +0200

    Always report invalid non-null MBR of universal face
    
    Even when no bbox is given to ValidateTopology.
    
    References #5766 in master branch (3.5.0dev)
    Includes regress test

diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index df71b5690..2b3c5bfa2 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -14,6 +14,8 @@
 --
 -- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
+-- #define POSTGIS_TOPOLOGY_DEBUG 1
+
 --
 -- Type returned by ValidateTopology
 --
@@ -26,8 +28,7 @@ CREATE TYPE topology.ValidateTopology_ReturnType AS (
 );
 
 --{
--- Return the exterior ring of a topology face
---
+-- Return the outermost ring of a topology face
 --
 CREATE OR REPLACE FUNCTION topology._ValidateTopologyGetFaceShellMaximalEdgeRing(atopology varchar, aface int)
 RETURNS GEOMETRY AS
@@ -35,6 +36,7 @@ $BODY$
 DECLARE
   sql TEXT;
   outsidePoint GEOMETRY;
+  leftmostEdge INT;
   shell GEOMETRY;
 BEGIN
 
@@ -53,38 +55,46 @@ BEGIN
   );
   EXECUTE sql USING aface INTO outsidePoint;
 
+  RAISE NOTICE 'Outside point of face %: %', aface, ST_AsText(outsidePoint);
+
+  IF outsidePoint IS NULL THEN
+    RETURN NULL;
+  END IF;
+
+  sql := format(
+    $$
+      SELECT
+        CASE WHEN left_face = $1
+        THEN
+          edge_id
+        ELSE
+          -edge_id
+        END ring_id
+      FROM %1$I.edge
+      WHERE left_face = $1 or right_face = $1
+      ORDER BY
+        geom <-> $2
+      LIMIT 1
+    $$,
+    atopology
+  );
+  EXECUTE sql USING aface, outsidePoint INTO leftmostEdge;
+
+  RAISE NOTICE 'Leftmost edge of face %: %', aface, leftmostEdge;
+
+  IF leftmostEdge IS NULL THEN
+    RETURN NULL;
+  END IF;
+
   sql := format(
     $$
       WITH
-      outside_point AS (
-        SELECT ST_Translate(
-          ST_StartPoint( ST_BoundingDiagonal(mbr) ),
-          -1,
-          -1
-        )
-        FROM %1$I.face
-        WHERE face_id = $1
-      ),
-      leftmost_edge AS (
-        SELECT
-          CASE WHEN left_face = $1
-          THEN
-            edge_id
-          ELSE
-            -edge_id
-          END ring_id
-        FROM %1$I.edge
-        WHERE left_face = $1 or right_face = $1
-        ORDER BY
-          geom <-> $2
-        LIMIT 1
-      ),
       edgering AS (
         SELECT *
         FROM
           topology.GetRingEdges(
             %1$L,
-            (SELECT ring_id FROM leftmost_edge)
+            $1
           )
       )
       SELECT
@@ -102,9 +112,9 @@ BEGIN
     atopology
   );
 
-  --RAISE DEBUG 'SQL: %', sql;
+  RAISE DEBUG 'SQL: %', sql;
 
-  EXECUTE sql USING aface, outsidePoint
+  EXECUTE sql USING leftmostEdge
   INTO shell;
 
   -- TODO: check if the ring is not closed
@@ -487,20 +497,15 @@ BEGIN
 
     found_rings := found_rings + 1;
 
-#ifdef POSTGIS_TOPOLOGY_DEBUG
-    RAISE DEBUG 'Ring % - faces:[%]',
-      rec.ring_id,
-      array_to_string(rec.side_faces, ',')
-    ;
-#endif /* POSTGIS_TOPOLOGY_DEBUG */
-
     -- Check that there's a single face advertised
     IF array_upper(rec.side_faces,1) != 1
     THEN --{
 
 #ifdef POSTGIS_TOPOLOGY_DEBUG
-      RAISE DEBUG 'Side faces found on ring %: %', rec.ring_id,
-       rec.side_faces;
+    RAISE DEBUG 'Ring % - mixed side faces:[%]',
+      rec.ring_id,
+      array_to_string(rec.side_faces, ',')
+    ;
 #endif /* POSTGIS_TOPOLOGY_DEBUG */
       retrec.error = 'mixed face labeling in ring';
       retrec.id1 = rec.ring_id;
@@ -539,6 +544,14 @@ BEGIN
       END IF;
     END IF;
 
+#ifdef POSTGIS_TOPOLOGY_DEBUG
+    RAISE DEBUG 'Ring % - side faces:[%] - %',
+      rec.ring_id,
+      array_to_string(rec.side_faces, ','),
+      CASE WHEN is_shell THEN 'shell (will end in shell_check)' ELSE 'hole' END
+    ;
+#endif /* POSTGIS_TOPOLOGY_DEBUG */
+
     IF is_shell THEN --{ It's a shell (CCW)
       -- Check that a single face is ever used
       --       for each distinct CCW ring (shell)
@@ -891,29 +904,47 @@ BEGIN
     f.face_id = sc.face_id
   ;
 
+#ifdef POSTGIS_TOPOLOGY_DEBUG
+  GET DIAGNOSTICS affected_rows = ROW_COUNT;
+  RAISE DEBUG 'Constructed geometries of % faces (found shells)',
+    affected_rows;
+#endif /* POSTGIS_TOPOLOGY_DEBUG */
+
   DROP TABLE pg_temp.shell_check;
 
+  --
+  -- Add to face_check any missing face whose mbr overlaps
+  -- the given one.
+  --
+  -- This is done to still be able to check MBR consistency
+  -- See https://trac.osgeo.org/postgis/ticket/5766#comment:6
+  --
+  INSERT INTO pg_temp.face_check
+  SELECT face_id,
+    CASE WHEN face_id = 0 THEN
+      NULL
+    ELSE
+      topology._ValidateTopologyGetFaceShellMaximalEdgeRing(toponame, face_id)
+    END as real_shell,
+    mbr
+  FROM face
+  WHERE ( bbox IS NULL OR mbr && bbox )
+  AND (
+    CASE WHEN invalid_faces IS NOT NULL THEN
+      NOT face_id = ANY(invalid_faces)
+    ELSE
+      TRUE
+    END
+  )
+  AND face_id NOT IN (
+    SELECT face_id FROM pg_temp.face_check
+  );
 
-  IF bbox IS NOT NULL
-  THEN --{
-    INSERT INTO pg_temp.face_check
-    SELECT face_id,
-      topology._ValidateTopologyGetFaceShellMaximalEdgeRing(toponame, face_id),
-      mbr
-    FROM face
-    WHERE mbr && bbox
-    AND (
-      CASE WHEN invalid_faces IS NOT NULL THEN
-        NOT face_id = ANY(invalid_faces)
-      ELSE
-        TRUE
-      END
-    )
-    AND face_id NOT IN (
-      SELECT face_id FROM pg_temp.face_check
-    )
-    ;
-  END IF; --}
+#ifdef POSTGIS_TOPOLOGY_DEBUG
+  GET DIAGNOSTICS affected_rows = ROW_COUNT;
+  RAISE DEBUG 'Added % missing faces in face_check table',
+    affected_rows;
+#endif /* POSTGIS_TOPOLOGY_DEBUG */
 
   -- Build a gist index on geom
   CREATE INDEX ON face_check USING gist (shell);
@@ -931,26 +962,53 @@ BEGIN
 
     affected_rows := affected_rows + 1;
 
-    IF rec.shell IS NULL OR ST_IsEmpty(rec.shell)
-    THEN
-      -- Face missing !
-      retrec.error := 'face has no rings';
-      retrec.id1 := rec.face_id;
-      retrec.id2 := NULL;
-      RETURN NEXT retrec;
-    END IF;
+    IF rec.face_id != 0 THEN -- {
 
-    IF NOT ST_Equals(rec.mbr, ST_Envelope(rec.shell))
-    THEN
+      -- Real face need have rings and matching MBR
+
+      IF rec.shell IS NULL OR ST_IsEmpty(rec.shell)
+      THEN
+        -- Face missing !
+        retrec.error := 'face has no rings';
+        retrec.id1 := rec.face_id;
+        retrec.id2 := NULL;
+        RETURN NEXT retrec;
+      END IF;
+
+      IF NOT ST_Equals(rec.mbr, ST_Envelope(rec.shell))
+      THEN
 #ifdef POSTGIS_TOPOLOGY_DEBUG
-      RAISE DEBUG 'MBR expected:% obtained:%', ST_AsEWKT(ST_Envelope(rec.shell)), ST_AsEWKT(ST_Envelope(rec.mbr));
+        RAISE DEBUG 'MBR expected:% obtained:%', ST_AsEWKT(ST_Envelope(rec.shell)), ST_AsEWKT(ST_Envelope(rec.mbr));
 #endif /* POSTGIS_TOPOLOGY_DEBUG */
-      -- Inconsistent MBR!
-      retrec.error := 'face has wrong mbr';
-      retrec.id1 := rec.face_id;
-      retrec.id2 := NULL;
-      RETURN NEXT retrec;
-    END IF;
+        -- Inconsistent MBR!
+        retrec.error := 'face has wrong mbr';
+        retrec.id1 := rec.face_id;
+        retrec.id2 := NULL;
+        RETURN NEXT retrec;
+      END IF;
+
+    ELSE --}{
+
+      -- Universal face need have no shell rings and NULL MBR
+
+      IF rec.shell IS NOT NULL OR NOT ST_IsEmpty(rec.shell)
+      THEN
+        retrec.error := 'universal face has shell rings';
+        retrec.id1 := rec.face_id;
+        retrec.id2 := NULL;
+        RETURN NEXT retrec;
+      END IF;
+
+      IF rec.mbr IS NOT NULL
+      THEN
+        -- TODO: make the message more specific about universal face ?
+        retrec.error := 'face has wrong mbr';
+        retrec.id1 := rec.face_id;
+        retrec.id2 := NULL;
+        RETURN NEXT retrec;
+      END IF;
+
+    END IF; --}
 
   END LOOP; --}
 
diff --git a/topology/test/regress/validatetopology.sql b/topology/test/regress/validatetopology.sql
index 8a23aed2e..173e4a5b1 100644
--- a/topology/test/regress/validatetopology.sql
+++ b/topology/test/regress/validatetopology.sql
@@ -206,5 +206,19 @@ SELECT '#5548.1', * FROM topology.ValidateTopology('t5548',
   ST_MakeEnvelope(2,2,4,4));
 ROLLBACK;
 
+-- See https://trac.osgeo.org/postgis/ticket/5766
+BEGIN;
+SELECT NULL FROM topology.CreateTopology('t5766');
+SELECT NULL FROM topology.TopoGeo_addPolygon('t5766', 'POLYGON((0 0, 1 2, 2 0,0 0))');
+UPDATE t5766.face SET mbr = ST_MakeEnvelope(0,0,2,2) WHERE face_id = 0; -- bbox matching leftmost ring
+SELECT '#5766.1', 'no bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766') v;
+SELECT '#5766.1', 'overlapping bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766', 'POINT(1 1)') v;
+SELECT '#5766.1', 'disjoint bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766', 'POINT(3 0)') v;
+UPDATE t5766.face SET mbr = ST_MakeEnvelope(0,0,1,2) WHERE face_id = 0; -- bbox not matching leftmost ring
+SELECT '#5766.2', 'no bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766') v;
+SELECT '#5766.2', 'overlapping bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766', 'POINT(1 1)') v;
+SELECT '#5766.2', 'disjoint bbox', (array_agg(v))[1] FROM topology.ValidateTopology('t5766', 'POINT(3 0)') v;
+ROLLBACK;
+
 SELECT NULL FROM topology.DropTopology('city_data');
 
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index 82242d0a4..da497ee20 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -40,3 +40,9 @@
 #5403.0|1
 #5548.0|1
 #5548.1|face without edges|1|
+#5766.1|no bbox|("face has wrong mbr",0,)
+#5766.1|overlapping bbox|("face has wrong mbr",0,)
+#5766.1|disjoint bbox|
+#5766.2|no bbox|("face has wrong mbr",0,)
+#5766.2|overlapping bbox|("face has wrong mbr",0,)
+#5766.2|disjoint bbox|

-----------------------------------------------------------------------

Summary of changes:
 topology/sql/manage/ValidateTopology.sql.in     | 206 +++++++++++++++---------
 topology/test/regress/validatetopology.sql      |  14 ++
 topology/test/regress/validatetopology_expected |   6 +
 3 files changed, 152 insertions(+), 74 deletions(-)


hooks/post-receive
-- 
PostGIS


More information about the postgis-tickets mailing list