Opened 19 months ago
Closed 18 months ago
#4814 closed defect (fixed)
GEOS roundtrip conversion drops empty components
Reported by: | strk | Owned by: | strk |
---|---|---|---|
Priority: | critical | Milestone: | PostGIS 3.1.1 |
Component: | postgis | Version: | 2.5.x |
Keywords: | Cc: |
Description
strk=# select ST_AsText(postgis_geos_noop('GEOMETRYCOLLECTION(POINT EMPTY, POINT(1 1))')); st_astext -------------------------------- GEOMETRYCOLLECTION(POINT(1 1)) (1 row) strk=# select ST_AsText('GEOMETRYCOLLECTION(POINT EMPTY, POINT(1 1))'); st_astext -------------------------------------------- GEOMETRYCOLLECTION(POINT EMPTY,POINT(1 1)) (1 row)
It looks our postgis→geos→postgis conversion is dropping EMPTY components. I think it shouldn't.
Found during work on #4813
Change History (18)
comment:1 by , 19 months ago
comment:4 by , 19 months ago
Letting EMPTY components get to GEOS revealed some bugs in GEOS. For example, this query crashes when run against GEOS-3.6.2:
SELECT ST_Intersects( 'LINESTRING(10 0,0 10)', 'MULTILINESTRING((10 -10,-10 10), EMPTY)' );
comment:6 by , 19 months ago
I filed https://trac.osgeo.org/geos/ticket/1085 for the crash. We'd have to decide if it's safer NOT to pass those EMPTY to GEOS yet, if that opens such can of worms…
comment:7 by , 19 months ago
It was actually my very first bug fix for PostGIS. The code can be removed after we stop supporting GEOS 3.8 and before, if the bug fixes will go into 3.9 - it's an explosive set of crashers.
comment:8 by , 19 months ago
Thanks Komzpa, your skipping EMPTY in #3579 was probably a good short-term solution but in the long run we want those crashes fixed upstream, in GEOS. I could revert [1c7c85c4/git] and [846d74b/git] to re-introduce the skip but this would then end up having different results from MakeValid depending on whether or not we use GEOS to perform the operation - see mail on https://lists.osgeo.org/pipermail/postgis-devel/2020-December/028710.html
comment:9 by , 19 months ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
This was done. GEOS-3.6.5 was released to fix the crash there.
comment:10 by , 19 months ago
Priority: | medium → critical |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Ubuntu Groovy + POSTGIS="3.1.0 5e2af69" [EXTENSION] PGSQL="130" GEOS="3.8.1-CAPI-1.13.3" SFCGAL="1.3.7" PROJ="7.1.0" GDAL="GDAL 3.1.3, released 2020/09/01" LIBXML="2.9.10" LIBJSON="0.15" LIBPROTOBUF="1.3.3" WAGYU="0.5.0 (Internal)" TOPOLOGY RASTER
Also tested on GEOS 3.9 and 3.10 snapshots:
02:00:17 [gis] > select ST_MakeValid('0106000020110F000000000000'); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Succeeded. Time: 9361,394 ms (00:09,361)
comment:12 by , 19 months ago
Looks like being a PostGIS, not GEOS, issue:
==12253== Invalid free() / delete / delete[] / realloc() ==12253== at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==12253== by 0x18D89A58: lwcollection_make_geos_friendly (lwgeom_geos_clean.c:345) ==12253== by 0x18D8A47A: lwgeom_make_valid (lwgeom_geos_clean.c:906) ==12253== by 0x18CAC1D1: ST_MakeValid (lwgeom_geos_clean.c:71) ==12253== by 0x348E55: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F02EB: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F1BB0: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F25D9: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x393960: expression_tree_mutator (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F2401: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x39405A: expression_tree_mutator (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F2401: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== Address 0x18908018 is 104 bytes inside a block of size 8,192 alloc'd ==12253== at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==12253== by 0x5BA367: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x5BF7AD: palloc (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x18D65A49: lwcollection_from_gserialized2_buffer (gserialized2.c:1413) ==12253== by 0x18D65A49: lwgeom_from_gserialized2_buffer (gserialized2.c:1492) ==12253== by 0x18D673C2: lwgeom_from_gserialized2 (gserialized2.c:1529) ==12253== by 0x18CAC1BD: ST_MakeValid (lwgeom_geos_clean.c:49) ==12253== by 0x348E55: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F02EB: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F1BB0: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F25D9: ??? (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x393960: expression_tree_mutator (in /usr/lib/postgresql/10/bin/postgres) ==12253== by 0x3F2401: ??? (in /usr/lib/postgresql/10/bin/postgres)
comment:16 by , 19 months ago
Milestone: | PostGIS 3.1.0 → PostGIS 3.1.1 |
---|
comment:17 by , 18 months ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:18 by , 18 months ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I think this is still fixed, not sure why it was reopened rather than filing a new bug for the crash (now also fixed). Please file a new ticket (maybe referencing this one) if there's another crash.
Version 2.5 did not have
postgis_geos_noop
function so I'm not sure it happens there as well