Opened 3 years ago

Closed 3 years ago

#4825 closed defect (fixed)

Inconsistent results when running SELECT queries with ST_MakeValid() on invalid geometry repeatedly

Reported by: neumann Owned by: pramsey
Priority: medium Milestone: PostGIS 3.1.1
Component: postgis Version: master
Keywords: Cc:

Description

There seems to be something weird (memory issue) when ST_MakeValid() is used repeatedly on broken geometries. The same query, when executed repeatedly, gives different results over time.

Attached is a small dump containing only broken geometries and here is a query for this data:

SELECT t_id,
       ST_AsText(geometrie),
       ST_AsText(ST_MakeValid(geometrie)), 
       ST_IsValidReason(geometrie), 
       ST_IsValidReason(ST_MakeValid(geometrie))
  FROM arp_npl.erschlssngsplnung_erschliessung_linienobjekt eel 
    WHERE ST_IsValid(geometrie) = FALSE;

When the database is restarted and the query is executed again repeatedly, it is initially "back to normal" until the pattern with the weird results repeats again.

Attachments (2)

st_make_valid_problems.backup (18.9 KB ) - added by neumann 3 years ago.
Dump containing a couple of records with broken geometries
st_make_valid_problems.2.backup (63.3 KB ) - added by neumann 3 years ago.
Plain text dump

Download all attachments as: .zip

Change History (15)

by neumann, 3 years ago

Dump containing a couple of records with broken geometries

comment:1 by neumann, 3 years ago

Here is a screen recording demonstrating the behaviour: https://www.carto.net/neumann/temp/st_make_valid_mem_problems.mp4

comment:2 by neumann, 3 years ago

Summary: Memory Leak wiht ST_MakeValid() and invalid geometry?Memory Leak with ST_MakeValid() and invalid geometry?

comment:3 by strk, 3 years ago

Can you please report the output of SELECT postgis_full_version() ?

What version of PostgreSQL was the backup produced with ? Because I cannot restore it with PostgreSQL 10 …

comment:4 by strk, 3 years ago

I can't restore with PostgreSQL-12 either. If I understand the message correctly the backup was taking with PostgreSQL-14 (!). Is there any chance you can produce an SQL dump instead, for ease of access ? Or a dump with an earlier version of PostgreSQL ? (ideally not newer than 10)

comment:5 by neumann, 3 years ago

Output of postgis_full_version():

POSTGIS="3.2.0dev 3.1.0rc1-30-ga4cc12b6f" [EXTENSION] PGSQL="130" GEOS="3.9.0-CAPI-1.16.2" PROJ="7.2.0" LIBXML="2.9.10" LIBJSON="0.13.1" LIBPROTOBUF="1.3.3" WAGYU="0.5.0 (Internal)"

Output of version():

PostgreSQL 13.1 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0, 64-bit

I'll try to create another dump.

by neumann, 3 years ago

Plain text dump

comment:6 by strk, 3 years ago

Running the query under valgrind against PotgreSQL-10, PostGIS-3.1.0rc1-17-g591a307ae and GEOS-3.10.0dev-CAPI-1.15.0 reports no problems. I suggest the following steps:

  1. Further reduce the test dataset by _removing_ records where ST_IsValid(geometrie) — does the problem persist after this step ?
  2. Further reduce the test dataset by ONLY including id and geom as the columns, and provide no defaults (also helps restoring the dump)
  3. Report here what differences you see in the first and subsequent runs of the query

I tried spotting differences in ST_MakeValid output using this approach:

CREATE TABLE test(id int, g geometry, vg1 geometry, vg2 geometry);
INSERT INTO test(id, g) SELECT t_id, geometrie FROM arp_npl.erschlssngsplnung_erschliessung_linienobjekt;
UPDATE test set vg1 = ST_MakeValid(g);
UPDATE test set vg2 = ST_MakeValid(g);
SELECT id FROM test WHERE NOT ST_Equals(vg1, vg2);

I get not results from last query, do you ?

comment:7 by strk, 3 years ago

Ok while I get no lines where ST_Equals() is false, between made-valid geometries, I do get 17 rows in which ST_OrderingEquals() is different. Sounds like some kind of wild pointer, which is hard to spot within PostgreSQL (due to memory pooling). I'll put the geometries under observation outside of PostgreSQL (in unit test for liblwgeom)

comment:8 by neumann, 3 years ago

ok - so it may not be a memory leak then. Good.

The problem I have is that geometries that had been detected as invalid later get labeled as valid in subsequent runs. See https://www.carto.net/neumann/temp/st_make_valid_mem_problems.mp4 where you see that there are fewer and fewer lines in the result set, until at the end there is only one record labeled as invalid.

I would expect that the "SELECT" statement in this ticket would always result in the same record set when running with the same data.

comment:9 by neumann, 3 years ago

Summary: Memory Leak with ST_MakeValid() and invalid geometry?Inconsistent results when running SELECT queries with ST_MakeValid() on invalid geometry repeatedly

comment:10 by Sandro Santilli <strk@…>, 3 years ago

In 6f04d19/git:

Copy input of ST_MakeValid before passing to lwgeom_make_valid

The lwgeom_make_valid function can change the input geometry.
References #4825

comment:11 by strk, 3 years ago

Please try with current master branch (lacks an automated testcase, still). Basically we were passing the input geometry to a mutator function (lwgeom_make_valid). I'm not sure WHY PostgreSQL writes the modified geometry back to disk (I would expect it NOT to do it) but this is what seems to be happening to me. The patch above makes sure we work on a copy rather than on the original geometry.

comment:12 by neumann, 3 years ago

Yes, I just compiled it and confirm that it works fine now ;-) Thanks a lot for fixing it!

And thanks for explaining what was causing the problem!

comment:13 by Sandro Santilli <strk@…>, 3 years ago

Resolution: fixed
Status: newclosed

In 22e3168/git:

Copy input of ST_MakeValid before passing to lwgeom_make_valid

The lwgeom_make_valid function can change the input geometry.
Closes #4825 in 3.1 branch (3.1.1dev)

Note: See TracTickets for help on using tickets.