Opened 5 months ago

Closed 5 months ago

Last modified 4 months ago

#5766 closed defect (fixed)

ValidateTopology(varchar toponame, geometry bbox) return error when sending with bbox, but not with out bbox

Reported by: Lars Aksel Opsahl Owned by: strk
Priority: medium Milestone: PostGIS 3.3.7
Component: topology Version: master
Keywords: Cc:

Description (last modified by Lars Aksel Opsahl)

When calling with boxx I het this error 'face has wrong mbr|0|'

topology.ValidateTopology('gronn_2023_v9_nosimlify_004','0103000020A21000000100000005000000FC60C227D47F09409A564F720FB64D40FC60C227D47F0940B6D8185120874E40EBF20689D0021D40B6D8185120874E40EBF20689D0021D409A564F720FB64D40FC60C227D47F09409A564F720FB64D40');

If I call without it returns no errors

SELECT 'ValidateTopology without bb', * FROM topology.ValidateTopology('gronn_2023_v9_nosimlify_004');


This test code can be found in

src/test/sql/regress/run_resolve_test.sh --nodrop  ./src/test/sql/regress/rog_overlay_test_13c.sql; \

Test add in this https://gitlab.com/nibioopensource/resolve-overlap-and-gap/-/commit/5584a4e781c847c28fb1f1ef48bf97d2c86d35e9 in branch https://gitlab.com/nibioopensource/resolve-overlap-and-gap/-/tree/performance_and_robustnes_spike_remove?ref_type=heads

Change History (14)

comment:1 by Lars Aksel Opsahl, 5 months ago

Summary: ValidateTopology(varchar toponame, geometry bbox) return error but not with outValidateTopology(varchar toponame, geometry bbox) return error when sending with bbox, but not with out bbox

comment:2 by Lars Aksel Opsahl, 5 months ago

Description: modified (diff)

comment:3 by strk, 5 months ago

Running the run_resolve_test.sh line you provided fails for me with: +ERROR: relation "test_13.gsk_2023_segmenter_renska" does not exist at character 4716

comment:4 by Lars Aksel Opsahl, 5 months ago

Sorry my mistake, forgot to add input data, added them now.

(CI fails until 3.5 is out)

comment:5 by strk, 5 months ago

Status: newassigned

Instructions missed the need to run make at least once before running the test, but I finally got it run and with the addition of -v I can see the error:

PostgreSQL 16.3 (Debian 16.3-1.pgdg120+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit
  Postgis 3.5.0dev - (3.4.0rc1-1172-g37fb070ef) - 2024-06-07 07:32:13
  scripts 3.5.0dev 3.4.0rc1-1172-g37fb070ef
  GEOS: 3.13.0dev-CAPI-1.19.0
  PROJ: 9.1.1

Running tests

 after-create-script src/test/sql/regress/../../../../resolve_overlap_and_gap-static.sql .. ok 
 after-create-script src/test/sql/regress/../../../../install-runtime-deps.sql .. ok 
 ./src/test/sql/regress/rog_overlay_test_13c ... failed (diff expected obtained: /tmp/pgis_reg/test_3_diff)
-----------------------------------------------------------------------------
--- ./src/test/sql/regress/rog_overlay_test_13c_expected        2024-07-26 12:43:59.396411133 +0200
+++ /tmp/pgis_reg/test_3_out    2024-07-26 15:50:16.054497640 +0200
@@ -1,3 +1,4 @@
+ValidateTopology with bb|face has wrong mbr|0|
 num surfaces total no simplify|15
 num surfaces total no simplify below 5.0|0
 num surfaces total|14
-----------------------------------------------------------------------------

comment:6 by strk, 5 months ago

The passed bbox covers all primitive elements, query can be made more generic with:

SELECT * FROM topology.ValidateTopology(
 'gronn_2023_v9_nosimlify_004',
 (select st_extent(geom) from gronn_2023_v9_nosimlify_004.edge)
);

Enabling debug shows that 15 faces are checked when bbox is NOT given while 16 faces are checked when bbox IS given.

Checking the face table shows that there is indeed a bounding box recorded for face0 which is really a validity error!

So we have a bug (the problem is NOT detected when no BBOX is given) and an improvement opportunity (the message could be clearer?).

In any case the validity is resolved by setting the mbr field of face=0 to null:

update gronn_2023_v9_nosimlify_004.face
 set mbr = null
 where face_id = 0;

comment:7 by strk, 5 months ago

PR fixing the false negative when bbox is not passed is here: https://git.osgeo.org/gitea/postgis/postgis/pulls/209

Rewording the face0 case would require documenting another message in the errors table of https://postgis.net/docs/ValidateTopology.html and I'm not sure it's worth it ("wrong MBR" is still valid in this case, after all)

comment:8 by strk, 5 months ago

Milestone: PostGIS 3.4.3PostGIS 3.3.7
Version: 3.4.xmaster

3.3 and 3.4 branches are also affected

comment:9 by Sandro Santilli <strk@…>, 5 months ago

In 8080637/git:

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

comment:10 by Sandro Santilli <strk@…>, 5 months ago

In 4c8093b/git:

Always report invalid non-null MBR of universal face

Even when no bbox is given to ValidateTopology.

References #5766 in 3.4 branch (3.4.3dev)
Includes regress test

comment:11 by Sandro Santilli <strk@…>, 5 months ago

Resolution: fixed
Status: assignedclosed

In d2a61b1a/git:

Always report invalid non-null MBR of universal face

Even when no bbox is given to ValidateTopology.

Closes #5766 in 3.3 branch (3.3.7dev)
Includes regress test

comment:12 by Lars Aksel Opsahl, 5 months ago

Thanks, I compiled latest master and that worked nice, but when trying to upgrading a existing database running master from June some time I got this error when upgrading.

NOTICE:  Updating extension postgis 3.5.0dev
ERROR:  could not find function "ST_RemoveIrrelevantPointsForView" in file "/usr/lib/postgresql/14/lib/postgis-3.so"
CONTEXT:  SQL statement "ALTER EXTENSION postgis UPDATE TO "ANY";ALTER EXTENSION postgis UPDATE TO "3.5.0dev""
PL/pgSQL function postgis_extensions_upgrade(text) line 88 at EXECUTE

Should I report this as bug ?

comment:13 by strk, 4 months ago

Yes please, file a separate ticket for that one.

Note: See TracTickets for help on using tickets.