Opened 3 months ago

Closed 3 months 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 months ago.
Dump containing a couple of records with broken geometries
st_make_valid_problems.2.backup (63.3 KB) - added by neumann 3 months ago.
Plain text dump

Download all attachments as: .zip

Change History (15)

Changed 3 months ago by neumann

Dump containing a couple of records with broken geometries

comment:1 Changed 3 months ago by neumann

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

comment:2 Changed 3 months ago by neumann

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

comment:3 Changed 3 months ago by strk

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 Changed 3 months ago by strk

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 Changed 3 months ago by neumann

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.

Changed 3 months ago by neumann

Plain text dump

comment:6 Changed 3 months ago by strk

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 Changed 3 months ago by strk

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 Changed 3 months ago by neumann

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 Changed 3 months ago by neumann

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

comment:10 Changed 3 months ago by Sandro Santilli <strk@…>

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 Changed 3 months ago by strk

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 Changed 3 months ago by neumann

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 Changed 3 months ago by Sandro Santilli <strk@…>

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.