[postgis-tickets] [SCM] PostGIS branch main updated. 3.1.0rc1-307-g30aeed6
git at osgeo.org
git at osgeo.org
Mon Jul 12 07:48:21 PDT 2021
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, main has been updated
via 30aeed6904b3b74eb26c5c4cf01e0e9c53a2b3fb (commit)
via 02084be1e2ff3635321e4650f0df683b40fc2347 (commit)
via bfb2794fff89048450b7f3f502066fe2d58153af (commit)
via e8c27e330ebd43f38045afb367114d0219ab6ce3 (commit)
via 5a4bde798d22250d76fe79571ad3ce3f26a5867f (commit)
via e0da8d5d20339aae366eec5f6959a7e3271269d5 (commit)
via 9fb6545e1c2e7d5fa9823848d48b4c57e3a8bea1 (commit)
via d011890dd21a54b191819c2319e9feb12955444f (commit)
via f99a5aa6a240d5d229142a68ebb318eb392979e1 (commit)
via fe51e6159178105886d57067ebc78cfb658fcdcb (commit)
from 5b4eff458245cb030f76a53ae4ae15c991d31357 (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 30aeed6904b3b74eb26c5c4cf01e0e9c53a2b3fb
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 16:13:16 2021 +0200
Precompute exterior ring of faces to check
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index 566a329..abea5e0 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -782,7 +782,11 @@ BEGIN
CREATE TEMP TABLE face_check ON COMMIT DROP AS
SELECT
face_id,
- topology.ST_GetFaceGeometry(toponame, face_id) AS geom,
+ ST_MakePolygon(
+ ST_ExteriorRing(
+ topology.ST_GetFaceGeometry(toponame, face_id)
+ )
+ ) AS shell,
mbr
FROM
face
@@ -802,10 +806,10 @@ BEGIN
;
-- Build a gist index on geom
- CREATE INDEX "face_check_gist" ON face_check USING gist (geom);
+ CREATE INDEX ON face_check USING gist (shell);
-- Build a btree index on id
- CREATE INDEX "face_check_bt" ON face_check (face_id);
+ CREATE INDEX ON face_check (face_id);
-- Scan the table looking for NULL geometries
-- or geometries with wrong MBR consistency
@@ -817,7 +821,7 @@ BEGIN
affected_rows := affected_rows + 1;
- IF rec.geom IS NULL OR ST_IsEmpty(rec.geom)
+ IF rec.shell IS NULL OR ST_IsEmpty(rec.shell)
THEN
-- Face missing !
retrec.error := 'face has no rings';
@@ -826,10 +830,10 @@ BEGIN
RETURN NEXT retrec;
END IF;
- IF NOT ST_Equals(rec.mbr, ST_Envelope(rec.geom))
+ 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.geom)), 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';
@@ -842,7 +846,36 @@ BEGIN
RAISE NOTICE 'Checked % faces', affected_rows;
+ -- Check edges are covered by their left-right faces (#4830)
+ RAISE NOTICE 'Checking for holes coverage';
+ affected_rows := 0;
+ FOR rec IN
+ SELECT * FROM hole_check
+ LOOP --{
+ SELECT f.face_id
+ FROM face_check f
+ WHERE rec.hole @ f.shell
+ AND _ST_Contains(f.shell, rec.hole)
+ ORDER BY ST_Area(f.shell) ASC
+ LIMIT 1
+ INTO rec2;
+
+ IF ( NOT FOUND AND rec.in_shell != 0 )
+ OR ( rec2.face_id != rec.in_shell )
+ THEN
+ retrec.error := 'hole not in advertised face';
+ retrec.id1 := rec.ring_id;
+ retrec.id2 := NULL;
+ RETURN NEXT retrec;
+ END IF;
+ affected_rows := affected_rows + 1;
+
+ END LOOP; --}
+
+ RAISE NOTICE 'Finished checking for coverage of % holes', affected_rows;
+
-- Check nodes have correct containing_face (#3233)
+ -- NOTE: relies on correct edge linking
RAISE NOTICE 'Checking for node containing_face correctness';
FOR rec IN
SELECT
@@ -904,33 +937,6 @@ BEGIN
END LOOP; --}
- -- Check edges are covered by their left-right faces (#4830)
- RAISE NOTICE 'Checking for holes coverage';
- affected_rows := 0;
- FOR rec IN
- SELECT * FROM hole_check
- LOOP --{
- SELECT f.face_id
- FROM face_check f
- WHERE rec.hole @ f.geom
- AND _ST_Contains(ST_MakePolygon(ST_ExteriorRing(f.geom)), rec.hole)
- ORDER BY ST_Area(f.geom) ASC
- LIMIT 1
- INTO rec2;
-
- IF ( NOT FOUND AND rec.in_shell != 0 )
- OR ( rec2.face_id != rec.in_shell )
- THEN
- retrec.error := 'hole not in advertised face';
- retrec.id1 := rec.ring_id;
- retrec.id2 := NULL;
- RETURN NEXT retrec;
- END IF;
- affected_rows := affected_rows + 1;
-
- END LOOP; --}
-
- RAISE NOTICE 'Finished checking for coverage of % holes', affected_rows;
DROP TABLE pg_temp.hole_check;
DROP TABLE pg_temp.face_check;
commit 02084be1e2ff3635321e4650f0df683b40fc2347
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 15:29:07 2021 +0200
Add NEWS item about ValidateTopology speedup
diff --git a/NEWS b/NEWS
index 3de871f..87eb4c1 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,8 @@ PostGIS 3.2.0
- #4933, topology.GetFaceByPoint will not work with topologies having invalid edge linking.
* Enhancements *
+ - #4950, Speed up checking containing_face for nodes in ValidateTopology
+ (Sandro Santilli)
- #4945, Multi-shell face check in ValidateTopology (Sandro Santilli)
- #4944, Side-location conflict check in ValidateTopology (Sandro Santilli)
- #3042, ValidateTopology check for edge linking (Sandro Santilli)
commit bfb2794fff89048450b7f3f502066fe2d58153af
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 15:21:38 2021 +0200
Speed up node's containing_face checking in ValidateTopology
Closes #4950
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index a2746d8..566a329 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -475,6 +475,7 @@ DECLARE
has_invalid_edge_linking BOOLEAN := false;
has_invalid_rings BOOLEAN := false;
search_path_backup text;
+ containing_face integer;
BEGIN
IF NOT EXISTS (
@@ -846,38 +847,61 @@ BEGIN
FOR rec IN
SELECT
n.node_id,
- -- in a corrupted topology multiple faces may contain the node
- min(f.face_id) face_id,
- -- multiple edges may contain the node
- min(e.edge_id) edge_id
+ n.geom geom,
+ n.containing_face,
+ e.edge_id
FROM node n
- LEFT JOIN face_check f ON ( ST_Contains(f.geom, n.geom) )
LEFT JOIN edge e ON (
e.start_node = n.node_id OR
e.end_node = n.node_id
)
WHERE
( bbox IS NULL OR n.geom && bbox )
- AND (
- (
- e.edge_id IS NULL AND (
- n.containing_face != f.face_id
- OR ( n.containing_face IS NULL AND f.face_id IS NOT NULL )
- )
- )
- OR ( n.containing_face IS NOT NULL AND e.edge_id IS NOT NULL)
- )
- GROUP BY n.node_id, n.containing_face
LOOP --{
- IF rec.edge_id IS NOT NULL THEN
- -- node is not really isolated
- retrec.error := 'not-isolated node has not-null containing_face';
- ELSE
- retrec.error := 'isolated node has wrong containing_face';
- END IF;
- retrec.id1 := rec.node_id;
- retrec.id2 := NULL; -- TODO: write expected containing_face here ?
- RETURN NEXT retrec;
+
+ IF rec.edge_id IS NOT NULL
+ THEN --{
+ -- Node is not isolated, make sure it
+ -- advertises itself as such
+ IF rec.containing_face IS NOT NULL
+ THEN --{
+ -- node is not really isolated
+ retrec.error := 'not-isolated node has not-null containing_face';
+ retrec.id1 := rec.node_id;
+ retrec.id2 := NULL;
+ RETURN NEXT retrec;
+ END IF; --}
+ ELSE -- }{
+ -- Node is isolated, make sure it
+ -- advertises itself as such
+ IF rec.containing_face IS NULL
+ THEN --{
+ -- isolated node advertises itself as non-isolated
+ retrec.error := 'isolated node has null containing_face';
+ retrec.id1 := rec.node_id;
+ retrec.id2 := NULL;
+ RETURN NEXT retrec;
+ ELSE -- }{
+ -- node is isolated and advertising a containing_face
+ -- now let's check it's really in contained by it
+ BEGIN
+ containing_face := topology.GetFaceContainingPoint(toponame, rec.geom);
+ EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'Got % (%)', SQLSTATE, SQLERRM;
+ retrec.error := format('got exception trying to find face containing node: %s', SQLERRM);
+ retrec.id1 := rec.node_id;
+ retrec.id2 := NULL;
+ RETURN NEXT retrec;
+ END;
+ IF containing_face != rec.containing_face THEN
+ retrec.error := 'isolated node has wrong containing_face';
+ retrec.id1 := rec.node_id;
+ retrec.id2 := NULL; -- TODO: write expected containing_face here ?
+ RETURN NEXT retrec;
+ END IF;
+ END IF; --}
+ END IF; --}
+
END LOOP; --}
-- Check edges are covered by their left-right faces (#4830)
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index 9f93d45..4b98f6d 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -4,7 +4,7 @@
#3233.1|---||
#3233.1|isolated node has wrong containing_face|2|
#3233.2|---||
-#3233.2|isolated node has wrong containing_face|2|
+#3233.2|isolated node has null containing_face|2|
#3233.3|---||
#3233.3|not-isolated node has not-null containing_face|1|
#3042|---||
commit e8c27e330ebd43f38045afb367114d0219ab6ce3
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 14:54:12 2021 +0200
Add notice about number of holes checked for containment
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index 64af5f3..a2746d8 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -749,7 +749,8 @@ BEGIN
RETURN NEXT retrec;
END LOOP; --}
- --- Validate edge linking
+ -- Validate edge linking
+ -- NOTE: relies on correct start_node/end_node on edges
FOR rec IN SELECT * FROM topology._ValidateTopologyEdgeLinking(bbox)
LOOP
RETURN next rec;
@@ -881,6 +882,7 @@ BEGIN
-- Check edges are covered by their left-right faces (#4830)
RAISE NOTICE 'Checking for holes coverage';
+ affected_rows := 0;
FOR rec IN
SELECT * FROM hole_check
LOOP --{
@@ -900,12 +902,14 @@ BEGIN
retrec.id2 := NULL;
RETURN NEXT retrec;
END IF;
+ affected_rows := affected_rows + 1;
END LOOP; --}
- DROP TABLE pg_temp.hole_check;
+ RAISE NOTICE 'Finished checking for coverage of % holes', affected_rows;
- DROP TABLE face_check;
+ DROP TABLE pg_temp.hole_check;
+ DROP TABLE pg_temp.face_check;
EXECUTE 'SET search_PATH TO ' || search_path_backup;
commit 5a4bde798d22250d76fe79571ad3ce3f26a5867f
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 14:07:15 2021 +0200
Update expected results from legacy invalid test
diff --git a/topology/test/regress/legacy_invalid_expected b/topology/test/regress/legacy_invalid_expected
index 6d69100..7111063 100644
--- a/topology/test/regress/legacy_invalid_expected
+++ b/topology/test/regress/legacy_invalid_expected
@@ -11,16 +11,8 @@ edge crosses edge|30|32
edge crosses node|1|23
edge crosses node|27|14
edge end node geometry mis-match|30|3
-edge not covered by both its side faces|27|
-edge not covered by both its side faces|28|
-edge not covered by both its side faces|30|
edge not simple|28|
edge start node geometry mis-match|30|4
-face has no rings|10|
-face has wrong mbr|3|
-face has wrong mbr|6|
-face overlaps face|2|12
-face within face|2|11
face without edges|10|
invalid edge|33|
invalid next_left_edge|2|3
@@ -37,16 +29,6 @@ invalid next_right_edge|27|-22
invalid next_right_edge|28|-30
invalid next_right_edge|29|7
invalid next_right_edge|33|31
-mixed face labeling in ring|-33|
-mixed face labeling in ring|-32|
-mixed face labeling in ring|-29|
-mixed face labeling in ring|29|
-mixed face labeling in ring|32|
-mixed face labeling in ring|33|
-not-isolated node has not-null containing_face|4|
-#4936|missing_count|4
-#4936|missing|face has no rings|10|
-#4936|missing|face overlaps face|2|12
-#4936|missing|face within face|2|11
+#4936|missing_count|1
#4936|missing|face without edges|10|
Topology 'invalid_topology' dropped
commit e0da8d5d20339aae366eec5f6959a7e3271269d5
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 13:45:43 2021 +0200
Rewrite test for hole containment to really focus on that check
References #4830
diff --git a/topology/test/regress/validatetopology.sql b/topology/test/regress/validatetopology.sql
index 7c1c824..ab67acb 100644
--- a/topology/test/regress/validatetopology.sql
+++ b/topology/test/regress/validatetopology.sql
@@ -45,79 +45,6 @@ SELECT '#3233.3', (ValidateTopology('t')).* UNION
SELECT '#3233.3', '---', null, null ORDER BY 1,2,3,4;
SELECT null from ( select topology.DropTopology('t') ) as dt;
--- Test edges containment in their side faces
--- See https://trac.osgeo.org/postgis/ticket/4684
-
-SELECT null from ( select topology.CreateTopology('t') ) as ct;
-SELECT null from ( select TopoGeo_addPolygon('t',
- 'POLYGON((0 0,10 0,10 10,0 10,0 0))' -- face 1
-) ) af;
-SELECT null from ( select TopoGeo_addPolygon('t',
- 'POLYGON((20 0,30 0,30 10,20 10,20 0))' -- face 2
-) ) af;
-SELECT null from ( select TopoGeo_addLineString('t',
- 'LINESTRING(5 5, 10 5)') -- edge inside faces, touching on endpoint
-) al;
-SELECT null from ( select TopoGeo_addLineString('t',
- 'LINESTRING(10 6, 5 6)') -- edge inside faces, touching on startpoint
-) al;
-SELECT null from ( select TopoGeo_addLineString('t',
- 'LINESTRING(10 5, 15 5)') -- edge outside faces, touching on startpoint
-) al;
-SELECT null from ( select TopoGeo_addLineString('t',
- 'LINESTRING(15 6, 10 6)') -- edge outside faces, touching on endpoint
-) al;
-SELECT null from ( select TopoGeo_addLineString('t',
- 'LINESTRING(5 8, 6 8)') -- edge inside face1, isolated
-) al;
-SELECT '#4830.0', (ValidateTopology('t')).* UNION
-SELECT '#4830.0', '---', null, null ORDER BY 1,2,3,4;
--- 1. Set wrong left_face for dangling inside edges
-BEGIN;
-UPDATE t.edge_data SET left_face = 2
- WHERE
- ST_Equals(geom, 'LINESTRING(5 5, 10 5)') OR
- ST_Equals(geom, 'LINESTRING(10 6, 5 6)');
-SELECT '#4830.1', (ValidateTopology('t')).* UNION
-SELECT '#4830.1', '---', null, null ORDER BY 1,2,3,4;
-ROLLBACK;
--- 2. Set wrong right_face for dangling inside edges
-BEGIN;
-UPDATE t.edge_data SET right_face = 2
- WHERE
- ST_Equals(geom, 'LINESTRING(5 5, 10 5)') OR
- ST_Equals(geom, 'LINESTRING(10 6, 5 6)');
-SELECT '#4830.2', (ValidateTopology('t')).* UNION
-SELECT '#4830.2', '---', null, null ORDER BY 1,2,3,4;
-ROLLBACK;
--- 3. Set wrong left_face for dangling outside edges
-BEGIN;
-UPDATE t.edge_data SET left_face = 2
- WHERE
- ST_Equals(geom, 'LINESTRING(10 5, 15 5)') OR
- ST_Equals(geom, 'LINESTRING(15 6, 10 6)');
-SELECT '#4830.3', (ValidateTopology('t')).* UNION
-SELECT '#4830.3', '---', null, null ORDER BY 1,2,3,4;
-ROLLBACK;
--- 4. Set wrong right_face for dangling outside edges
-BEGIN;
-UPDATE t.edge_data SET right_face = 2
- WHERE
- ST_Equals(geom, 'LINESTRING(10 5, 15 5)') OR
- ST_Equals(geom, 'LINESTRING(15 6, 10 6)');
-SELECT '#4830.4', (ValidateTopology('t')).* UNION
-SELECT '#4830.4', '---', null, null ORDER BY 1,2,3,4;
-ROLLBACK;
--- 5. Set universal face on both sides of internal edge
-BEGIN;
-UPDATE t.edge_data SET left_face = 0, right_face = 0
- WHERE
- ST_Equals(geom, 'LINESTRING(5 8, 6 8)');
-SELECT '#4830.5', (ValidateTopology('t')).* UNION
-SELECT '#4830.5', '---', null, null ORDER BY 1,2,3,4;
-ROLLBACK;
-SELECT null from ( select topology.DropTopology('t') ) as dt;
-
-------------------------------------------------------------
-- Following tests will use the city_data topology as a base
-------------------------------------------------------------
@@ -179,5 +106,44 @@ SELECT '#4945', '---', null, null
ORDER BY 1,2,3,4;
ROLLBACK;
+-- Test outer face labeling
+-- See https://trac.osgeo.org/postgis/ticket/4684
+
+-- 1. Set wrong outer face for isolated inside edges
+BEGIN;
+UPDATE city_data.edge_data SET left_face = 2, right_face = 2
+ WHERE edge_id = 25;
+SELECT '#4830.1', (ValidateTopology('city_data')).* UNION
+SELECT '#4830.1', '---', null, null ORDER BY 1,2,3,4;
+ROLLBACK;
+-- 2. Set wrong outer face for isolated outside edge
+BEGIN;
+UPDATE city_data.edge_data SET left_face = 2, right_face = 2
+ WHERE edge_id IN (4, 5);
+SELECT '#4830.2', (ValidateTopology('city_data')).* UNION
+SELECT '#4830.2', '---', null, null ORDER BY 1,2,3,4;
+ROLLBACK;
+-- 3. Set wrong outer face for face hole
+BEGIN;
+UPDATE city_data.edge_data SET right_face = 9
+ WHERE edge_id = 26;
+SELECT '#4830.3', (ValidateTopology('city_data')).* UNION
+SELECT '#4830.3', '---', null, null ORDER BY 1,2,3,4;
+ROLLBACK;
+-- 4. Set universal outer face for isolated edge inside a face
+BEGIN;
+UPDATE city_data.edge_data SET left_face = 0, right_face = 0
+ WHERE edge_id = 25;
+SELECT '#4830.4', (ValidateTopology('city_data')).* UNION
+SELECT '#4830.4', '---', null, null ORDER BY 1,2,3,4;
+ROLLBACK;
+-- 5. Set universal outer face for face hole
+BEGIN;
+UPDATE city_data.edge_data SET right_face = 0
+ WHERE edge_id = 26;
+SELECT '#4830.5', (ValidateTopology('city_data')).* UNION
+SELECT '#4830.5', '---', null, null ORDER BY 1,2,3,4;
+ROLLBACK;
+
SELECT NULL FROM topology.DropTopology('city_data');
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index ce8cadd..9f93d45 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -7,17 +7,6 @@
#3233.2|isolated node has wrong containing_face|2|
#3233.3|---||
#3233.3|not-isolated node has not-null containing_face|1|
-#4830.0|---||
-#4830.1|---||
-#4830.1|mixed face labeling in ring|-6|
-#4830.2|---||
-#4830.2|mixed face labeling in ring|-6|
-#4830.3|---||
-#4830.3|mixed face labeling in ring|-8|
-#4830.4|---||
-#4830.4|mixed face labeling in ring|-8|
-#4830.5|---||
-#4830.5|hole not in advertised face|-9|
#3042|---||
#3042|invalid next_left_edge|3|-3
#3042|invalid next_left_edge|9|19
@@ -32,3 +21,13 @@
#4944|mixed face labeling in ring|-19|
#4945|---||
#4945|face has multiple shells|3|
+#4830.1|---||
+#4830.1|hole not in advertised face|-25|
+#4830.2|---||
+#4830.2|hole not in advertised face|-5|
+#4830.3|---||
+#4830.3|hole not in advertised face|-26|
+#4830.4|---||
+#4830.4|hole not in advertised face|-25|
+#4830.5|---||
+#4830.5|hole not in advertised face|-26|
commit 9fb6545e1c2e7d5fa9823848d48b4c57e3a8bea1
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 02:45:15 2021 +0200
Report number of shells and holes, report non-closed rings
The non-closed ring error needs a testcase, which I think should be
possibly to write by using a limiting bounding box so that edge
linking succeeds but ring-checking fail (invalidity of the ring
outside the bbox)
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index 8beabb6..64af5f3 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -236,6 +236,8 @@ DECLARE
affected_rows integer;
ring_poly GEOMETRY;
is_shell BOOLEAN;
+ found_shells INT := 0;
+ found_holes INT := 0;
BEGIN
RAISE NOTICE 'Gathering edges for side faces labeling';
@@ -382,22 +384,20 @@ BEGIN
ELSE --}{
- -- Save ring that a single face is ever used
- -- for each distinct CCW ring (shell)
- -- NOTE: multiple CW rings (holes) can exist for a given face
--RAISE DEBUG 'Ring geom: %', ST_AsTexT(rec.ring_geom);
IF NOT ST_Equals(
ST_StartPoint(rec.ring_geom),
ST_EndPoint(rec.ring_geom)
)
THEN --{
--- -- This should have been reported before,
--- -- on the edge linking check
--- retrec.error = 'non-closed ring';
--- retrec.id1 = nextEdge;
--- retrec.id2 = NULL;
--- RETURN NEXT retrec;
+ -- This should have been reported before,
+ -- on the edge linking check
+ retrec.error = 'non-closed ring';
+ retrec.id1 = nextEdge;
+ retrec.id2 = NULL;
+ RETURN NEXT retrec;
ELSE --}{
+ -- Ring is valid, save it.
is_shell := false;
IF ST_NPoints(rec.ring_geom) > 3 THEN
ring_poly := ST_MakePolygon(rec.ring_geom);
@@ -405,12 +405,15 @@ BEGIN
is_shell := true;
END IF;
END IF;
- IF is_shell THEN
+ IF is_shell THEN --{ It's a shell (CCW)
+ -- Check that a single face is ever used
+ -- for each distinct CCW ring (shell)
BEGIN
INSERT INTO shell_check VALUES (
rec.side_faces[1],
ring_poly
);
+ found_shells := found_shells + 1;
EXCEPTION WHEN unique_violation THEN
retrec.error = 'face has multiple shells';
retrec.id1 = rec.side_faces[1];
@@ -418,11 +421,13 @@ BEGIN
RETURN NEXT retrec;
END;
ELSE -- }{ It's an hole (CW)
+ -- NOTE: multiple CW rings (holes) can exist for a given face
INSERT INTO hole_check VALUES (
nextEdge,
rec.ring_geom,
rec.side_faces[1]
);
+ found_holes := found_holes + 1;
END IF; --} hole
END IF; --}
@@ -435,7 +440,8 @@ BEGIN
END LOOP; --}
- RAISE NOTICE 'Completed checking % rings', affected_rows;
+ RAISE NOTICE 'Completed computing % rings, found % valid shells and % holes',
+ affected_rows, found_shells, found_holes;
DROP TABLE pg_temp.side_label_check_edge;
DROP TABLE pg_temp.shell_check;
commit d011890dd21a54b191819c2319e9feb12955444f
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Jul 12 02:20:55 2021 +0200
Early exit from ValidateTopology when it makes sense
Does not make sense to validate rings if edge linking is invalid,
nor to validate containement if rings are invalid...
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index ce5d788..8beabb6 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -466,6 +466,8 @@ DECLARE
affected_rows integer;
invalid_edges integer[];
invalid_faces integer[];
+ has_invalid_edge_linking BOOLEAN := false;
+ has_invalid_rings BOOLEAN := false;
search_path_backup text;
BEGIN
@@ -742,10 +744,26 @@ BEGIN
END LOOP; --}
--- Validate edge linking
- RETURN QUERY SELECT * FROM topology._ValidateTopologyEdgeLinking(bbox);
+ FOR rec IN SELECT * FROM topology._ValidateTopologyEdgeLinking(bbox)
+ LOOP
+ RETURN next rec;
+ has_invalid_edge_linking := true;
+ END LOOP;
+
+ IF has_invalid_edge_linking THEN
+ RETURN; -- does not make sense to continue
+ END IF;
--- Validate edge rings
- RETURN QUERY SELECT * FROM topology._ValidateTopologyRings(bbox);
+ FOR rec IN SELECT * FROM topology._ValidateTopologyRings(bbox)
+ LOOP
+ RETURN next rec;
+ has_invalid_rings := true;
+ END LOOP;
+
+ IF has_invalid_rings THEN
+ RETURN; -- does not make sense to continue
+ END IF;
-- Now create a temporary table to construct all face geometries
-- for checking their consistency
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index 733cca4..ce8cadd 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -27,14 +27,6 @@
#3042|invalid next_right_edge|3|2
#3042|invalid next_right_edge|19|-10
#3042|invalid next_right_edge|25|25
-#3042|mixed face labeling in ring|-22|
-#3042|mixed face labeling in ring|-21|
-#3042|mixed face labeling in ring|-20|
-#3042|mixed face labeling in ring|-10|
-#3042|mixed face labeling in ring|-9|
-#3042|mixed face labeling in ring|-6|
-#3042|mixed face labeling in ring|-3|
-#3042|mixed face labeling in ring|19|
#4944|---||
#4944|mixed face labeling in ring|-21|
#4944|mixed face labeling in ring|-19|
commit f99a5aa6a240d5d229142a68ebb318eb392979e1
Author: Sandro Santilli <strk at kbt.io>
Date: Sun Jul 11 23:17:09 2021 +0200
Check rings, rather than edges, for containment
Drop face-in-face, face-overlaps-face
All previous errors should be now intercepted,
only printed in a different way
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index d1ad393..ce5d788 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -223,6 +223,8 @@ LANGUAGE 'plpgsql' VOLATILE;
--
-- NOTE: assumes search_path was set before calling this function
--
+-- Creates a pg_temp.hole_check table
+--
CREATE OR REPLACE FUNCTION topology._ValidateTopologyRings(bbox geometry DEFAULT NULL)
RETURNS SETOF topology.ValidateTopology_ReturnType
AS --{
@@ -265,6 +267,7 @@ BEGIN
);
CREATE TEMP TABLE hole_check (
+ ring_id int,
hole geometry, -- linestring
in_shell int
);
@@ -416,6 +419,7 @@ BEGIN
END;
ELSE -- }{ It's an hole (CW)
INSERT INTO hole_check VALUES (
+ nextEdge,
rec.ring_geom,
rec.side_faces[1]
);
@@ -436,12 +440,6 @@ BEGIN
DROP TABLE pg_temp.side_label_check_edge;
DROP TABLE pg_temp.shell_check;
- -- TODO: Check that holes really belong to the advertised shell
- -- (instead of dropping the table)
- -- NOTE: when limited by bounding box we might be missing the
- -- shell belonging to the most external holes, so for
- -- those we would need to
- DROP TABLE pg_temp.hole_check;
END;
@@ -749,11 +747,11 @@ BEGIN
--- Validate edge rings
RETURN QUERY SELECT * FROM topology._ValidateTopologyRings(bbox);
-
-- Now create a temporary table to construct all face geometries
-- for checking their consistency
RAISE NOTICE 'Constructing geometry of all faces';
+ -- TODO: only construct exterior ring
CREATE TEMP TABLE face_check ON COMMIT DROP AS
SELECT
@@ -814,43 +812,6 @@ BEGIN
RETURN NEXT retrec;
END IF;
- FOR rec2 IN
- SELECT
- geom,
- face_id,
- ST_Relate(rec.geom, geom) as im
- FROM
- face_check
- WHERE face_id > rec.face_id
- AND geom && rec.geom
- LOOP --{
-
- -- Face overlap
- IF ST_RelateMatch(rec2.im, 'T*T***T**') THEN
- retrec.error = 'face overlaps face';
- retrec.id1 = rec.face_id;
- retrec.id2 = rec2.face_id;
- RETURN NEXT retrec;
- END IF;
-
- -- Face 1 is within face 2
- IF ST_RelateMatch(rec2.im, 'T*F**F***') THEN
- retrec.error = 'face within face';
- retrec.id1 = rec.face_id;
- retrec.id2 = rec2.face_id;
- RETURN NEXT retrec;
- END IF;
-
- -- Face 1 contains face 2
- IF ST_RelateMatch(rec2.im, 'T*****FF*') THEN
- retrec.error = 'face within face';
- retrec.id1 = rec.face_id;
- retrec.id2 = rec2.face_id;
- RETURN NEXT retrec;
- END IF;
-
- END LOOP; --}
-
END LOOP; --}
RAISE NOTICE 'Checked % faces', affected_rows;
@@ -894,103 +855,32 @@ BEGIN
RETURN NEXT retrec;
END LOOP; --}
- RAISE NOTICE 'Checking for edges coverage';
-
-- Check edges are covered by their left-right faces (#4830)
- RAISE NOTICE 'Checking for edges coverage';
- FOR rec IN --{
- WITH
- face_coverage AS (
- SELECT f.face_id, array_agg(e.edge_id) covered_edges
- FROM face_check f, edge_data e
- WHERE ST_Covers(f.geom, e.geom)
- GROUP BY f.face_id
- ),
- edges_to_check AS (
- SELECT
- e.*
- FROM edge_data e
- WHERE ( bbox IS NULL OR e.geom && bbox )
- AND e.edge_id NOT IN (
- SELECT unnest(invalid_edges)
- )
- ),
- edge_coverage AS (
- SELECT
- e.edge_id,
- e.left_face,
- e.right_face,
- array_agg(DISTINCT fc.face_id)
- filter (WHERE fc.face_id IS NOT NULL) faces_covering
- FROM edges_to_check e
- LEFT JOIN face_coverage fc ON (
- e.edge_id = ANY(fc.covered_edges)
- )
- GROUP BY edge_id, left_face, right_face
- )
- SELECT *
- FROM edge_coverage
- WHERE
- -- Edges dangling in universal face needs be
- -- NOT covered by any face
- (
- left_face = 0 AND
- right_face = 0 AND
- faces_covering IS NOT NULL
- )
- OR
- (
- left_face != 0 AND
- (
- faces_covering IS NULL OR
- NOT left_face = ANY(faces_covering)
- )
- )
- OR
- (
- right_face != 0 AND
- (
- faces_covering IS NULL OR
- NOT right_face = ANY(faces_covering)
- )
- )
- OR
- (
- (
- ( right_face = 0 AND left_face != 0 )
- OR
- ( right_face != 0 AND left_face = 0 )
- )
- AND
- (
- faces_covering IS NULL OR
- array_upper(faces_covering, 1) != 1
- )
- )
- ORDER BY edge_id
- LOOP --}{
- retrec.id1 := rec.edge_id;
- retrec.id2 := NULL;
- IF rec.left_face = 0
- AND rec.right_face = 0
- THEN
- retrec.error := 'edge covered by some face has universal face on both sides';
- --retrec.error := format('edge covered by faces %s has universal face on both sides', rec.faces_covering);
- RETURN NEXT retrec;
- ELSE
- retrec.error := 'edge not covered by both its side faces';
+ RAISE NOTICE 'Checking for holes coverage';
+ FOR rec IN
+ SELECT * FROM hole_check
+ LOOP --{
+ SELECT f.face_id
+ FROM face_check f
+ WHERE rec.hole @ f.geom
+ AND _ST_Contains(ST_MakePolygon(ST_ExteriorRing(f.geom)), rec.hole)
+ ORDER BY ST_Area(f.geom) ASC
+ LIMIT 1
+ INTO rec2;
+
+ IF ( NOT FOUND AND rec.in_shell != 0 )
+ OR ( rec2.face_id != rec.in_shell )
+ THEN
+ retrec.error := 'hole not in advertised face';
+ retrec.id1 := rec.ring_id;
+ retrec.id2 := NULL;
RETURN NEXT retrec;
--- IF rec.left_face != 0 AND NOT rec.left_face_covered THEN
--- retrec.error := 'edge not covered by its left face';
--- RETURN NEXT retrec;
--- END IF;
--- IF rec.right_face != 0 AND NOT rec.right_face_covered THEN
--- retrec.error := 'edge not covered by its right face';
--- RETURN NEXT retrec;
--- END IF;
- END IF;
+ END IF;
+
END LOOP; --}
+ DROP TABLE pg_temp.hole_check;
+
DROP TABLE face_check;
EXECUTE 'SET search_PATH TO ' || search_path_backup;
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index e2e9256..733cca4 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -9,23 +9,15 @@
#3233.3|not-isolated node has not-null containing_face|1|
#4830.0|---||
#4830.1|---||
-#4830.1|edge not covered by both its side faces|4|
-#4830.1|edge not covered by both its side faces|6|
#4830.1|mixed face labeling in ring|-6|
#4830.2|---||
-#4830.2|edge not covered by both its side faces|4|
-#4830.2|edge not covered by both its side faces|6|
#4830.2|mixed face labeling in ring|-6|
#4830.3|---||
-#4830.3|edge not covered by both its side faces|7|
-#4830.3|edge not covered by both its side faces|8|
#4830.3|mixed face labeling in ring|-8|
#4830.4|---||
-#4830.4|edge not covered by both its side faces|7|
-#4830.4|edge not covered by both its side faces|8|
#4830.4|mixed face labeling in ring|-8|
#4830.5|---||
-#4830.5|edge covered by some face has universal face on both sides|9|
+#4830.5|hole not in advertised face|-9|
#3042|---||
#3042|invalid next_left_edge|3|-3
#3042|invalid next_left_edge|9|19
commit fe51e6159178105886d57067ebc78cfb658fcdcb
Author: Sandro Santilli <strk at kbt.io>
Date: Fri Jul 9 18:12:35 2021 +0200
Refactor multi-shell check to use less statements
Also store shells and holes in different tables, to reuse
them for later checks.
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index a6eb0f5..d1ad393 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -259,11 +259,14 @@ BEGIN
CREATE INDEX ON side_label_check_edge (edge_id);
- CREATE TEMP TABLE ring_check (
- ring_id int,
- ring_geom geometry, -- polygon for shell, line for hole
- is_shell boolean,
- bound_face int
+ CREATE TEMP TABLE shell_check (
+ face_id int PRIMARY KEY,
+ shell geometry -- polygon
+ );
+
+ CREATE TEMP TABLE hole_check (
+ hole geometry, -- linestring
+ in_shell int
);
RAISE NOTICE 'Checking edge rings';
@@ -399,16 +402,24 @@ BEGIN
is_shell := true;
END IF;
END IF;
- INSERT INTO ring_check VALUES (
- nextEdge,
- CASE WHEN is_shell THEN
- ring_poly
- ELSE
- rec.ring_geom
- END,
- is_shell,
- rec.side_faces[1]
- );
+ IF is_shell THEN
+ BEGIN
+ INSERT INTO shell_check VALUES (
+ rec.side_faces[1],
+ ring_poly
+ );
+ EXCEPTION WHEN unique_violation THEN
+ retrec.error = 'face has multiple shells';
+ retrec.id1 = rec.side_faces[1];
+ retrec.id2 = NULL;
+ RETURN NEXT retrec;
+ END;
+ ELSE -- }{ It's an hole (CW)
+ INSERT INTO hole_check VALUES (
+ rec.ring_geom,
+ rec.side_faces[1]
+ );
+ END IF; --} hole
END IF; --}
END IF; --}
@@ -419,32 +430,18 @@ BEGIN
END LOOP; --}
- -- Check if any face has multiple shells
- RAISE NOTICE 'Checking multi-shell faces';
- FOR rec IN
- SELECT
- rc.bound_face,
- count(rc.ring_id)
- FROM ring_check rc
- WHERE rc.is_shell
- GROUP BY rc.bound_face
- HAVING count(rc.ring_id) > 1
- LOOP
- retrec.error = 'face has multiple shells';
- retrec.id1 = rec.bound_face;
- retrec.id2 = NULL;
- RETURN NEXT retrec;
- END LOOP;
+
+ RAISE NOTICE 'Completed checking % rings', affected_rows;
+
+ DROP TABLE pg_temp.side_label_check_edge;
+ DROP TABLE pg_temp.shell_check;
-- TODO: Check that holes really belong to the advertised shell
+ -- (instead of dropping the table)
-- NOTE: when limited by bounding box we might be missing the
-- shell belonging to the most external holes, so for
-- those we would need to
-
- RAISE NOTICE 'Completed checking % rings', affected_rows;
-
- DROP TABLE pg_temp.side_label_check_edge;
- DROP TABLE pg_temp.ring_check;
+ DROP TABLE pg_temp.hole_check;
END;
@@ -897,6 +894,8 @@ BEGIN
RETURN NEXT retrec;
END LOOP; --}
+ RAISE NOTICE 'Checking for edges coverage';
+
-- Check edges are covered by their left-right faces (#4830)
RAISE NOTICE 'Checking for edges coverage';
FOR rec IN --{
-----------------------------------------------------------------------
Summary of changes:
NEWS | 2 +
topology/sql/manage/ValidateTopology.sql.in | 359 ++++++++++--------------
topology/test/regress/legacy_invalid_expected | 20 +-
topology/test/regress/validatetopology.sql | 112 +++-----
topology/test/regress/validatetopology_expected | 39 +--
5 files changed, 206 insertions(+), 326 deletions(-)
hooks/post-receive
--
PostGIS
More information about the postgis-tickets
mailing list