Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#4770 closed defect (fixed)

[raster] Calling st_union() with WINDOW PARTITION crashes server

Reported by: robins Owned by: robe
Priority: blocker Milestone: PostGIS 3.0.4
Component: raster Version: 3.0.x
Keywords: st_union Cc:

Description

Brief

Calling st_union() with WINDOW PARTITION crashes the server.

Repro

WITH x AS (
  SELECT * FROM (VALUES ('A0006', 300), ('A0006', 302)) t(a,b)
)
SELECT 
 PUBLIC.st_union(NULL::raster, NULL::_unionarg) OVER (PARTITION BY a ORDER BY b)
FROM x;

SQL Output

db=> SELECT version();
-[ RECORD 1 ]---------------------------------------------------------------------------------------------------
version | PostgreSQL 13.0 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6), 64-bit

db=> SELECT postgis_full_version();
-[ RECORD 1 ]--------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
postgis_full_version | POSTGIS="3.0.2 2fb2a18" [EXTENSION] PGSQL="130" GEOS="3.8.0-CAPI-1.13.1 " PROJ="Rel. 5.2.0, September 15th, 2018" GDAL="GDAL 2.4.4, released 2020/01/08" LIBXML="2.9.1" LIBJSON="0.13.1" LIBPROTOBUF="1.3.2" WAGYU="0.4.3 (Internal)" TOPOLOGY RASTER

db=> WITH x AS (
db(>   SELECT * FROM (VALUES ('A0006', 300), ('A0006', 302)) t(a,b)
db(> )
db-> SELECT
db->  PUBLIC.st_union(NULL::raster, NULL::_unionarg) OVER (PARTITION BY a ORDER BY b)
db-> FROM x;
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: sFailed.
!?>

Change History (14)

comment:1 by Algunenano, 4 years ago

Component: postgisraster
Owner: changed from pramsey to dustymugs

comment:2 by Algunenano, 4 years ago

Summary: Calling st_union() with WINDOW PARTITION crashes server[raster] Calling st_union() with WINDOW PARTITION crashes server

comment:3 by robe, 4 years ago

Owner: changed from dustymugs to robe

comment:4 by robe, 4 years ago

This appears to be breaking at line #2437

https://git.osgeo.org/gitea/postgis/postgis/src/branch/master/raster/rt_pg/rtpg_mapalgebra.c#L2437

if (itrset == NULL) {

Doesn't make any sense to me how that could fail, but that is what gdb is telling me is the crashing line

	itrset = palloc(sizeof(struct rt_iterator_t) * 2);
	if (itrset == NULL) {

		rtpg_union_arg_destroy(iwr);
		if (raster != NULL) {
			rt_raster_destroy(raster);
			PG_FREE_IF_COPY(pgraster, 1);
		}

		MemoryContextSwitchTo(oldcontext);
		elog(ERROR, "RASTER_union_transfn: Could not allocate memory for iterator arguments");
		PG_RETURN_NULL();
	}


comment:5 by robe, 4 years ago

Milestone: PostGIS 3.1.0PostGIS 3.0.4

Going to push this to 3.0.4 to move out of 3.1.0 way and also since I think it is an issue in 3.0.4 too so should be fixed there as well.

comment:6 by robins, 4 years ago

2 other Tickets, with similar (WINDOW PARITION) crashes… may be they're related.

https://trac.osgeo.org/postgis/ticket/4916 https://trac.osgeo.org/postgis/ticket/4724

comment:7 by robe, 3 years ago

Priority: mediumblocker

comment:8 by pramsey, 3 years ago

It's very odd, the implementations of the transfns between union(geom) and union(rast) is functionally identical, and the problem is in fact that in the raster case pgsql itself is calling the transfn in a bogus state… promising a populated aggcontext while providing an empty one. There's something very odd going on in an interaction with pgsql.

Smaller reproduction:

SELECT 
 PUBLIC.st_union(NULL::raster) OVER (ORDER BY b)
FROM (VALUES (300), (302)) t(b);

comment:9 by pramsey, 3 years ago

The problem appears to be the finalfn freeing the aggregate state object. When I add a pfree(state) to the (working) st_union(geometry) aggregate, I can induce the same crashes. Similarly when I remove the pfree from the raster finalfn, the crashes go away.

The only concern I have about applying the fix over the various known cases is that I don't understand the mechanism behind leaving the state object intact… Magic Occurs! As far as I can tell, it's the correct magic, since the windowed results for the geometry union SEEM correct?

WITH w AS (
  SELECT 
    ST_Union(g) OVER (PARTITION BY b ORDER BY a) AS g
  FROM (VALUES ('POINT(0 0)'::geometry, 'A0006', 300), ('POINT(1 1)'::geometry, 'A0006', 302)) t(g, a, b)
)
SELECT ST_AsText(g) FROM w;

 st_astext  
------------
 POINT(0 0)
 POINT(1 1)

or with the partition wider…

WITH w AS (
  SELECT 
    ST_Union(g) OVER (PARTITION BY a ORDER BY b) AS g
  FROM (VALUES ('POINT(0 0)'::geometry, 'A0006', 300), ('POINT(1 1)'::geometry, 'A0006', 302)) t(g, a, b)
)
SELECT ST_AsText(g) FROM w;

      st_astext      
---------------------
 POINT(0 0)
 MULTIPOINT(0 0,1 1)

I'm hampered by not being 100% sure that the expected windowed results are from ST_Union(geometry) with PARTITION BY and ORDER BY are.

comment:10 by pramsey, 3 years ago

Looking at some windows on numbers it looks like we are doing logically the same thing as the built-ins, so I guess this fix can go in.

WITH w AS (
  SELECT 
    ST_Union(g) OVER (PARTITION BY a ORDER BY b) AS g,
    Sum(b) OVER (PARTITION BY a ORDER BY b) AS s
  FROM (VALUES ('POINT(0 0)'::geometry, 'A0006', 300), ('POINT(1 1)'::geometry, 'A0006', 302)) t(g, a, b)
)
SELECT ST_AsText(g), s FROM w;

      st_astext      |  s  
---------------------+-----
 POINT(0 0)          | 300
 MULTIPOINT(0 0,1 1) | 602

comment:11 by Paul Ramsey <pramsey@…>, 3 years ago

In ef2995b/git:

Fix crashes in aggregate functions, references #4916, #4770, #4724, #4916

comment:12 by Paul Ramsey <pramsey@…>, 3 years ago

In 22dec99/git:

Fix crashes in aggregate functions, references #4916, #4770, #4724, #4916

comment:13 by pramsey, 3 years ago

Resolution: fixed
Status: newclosed

comment:14 by Paul Ramsey <pramsey@…>, 3 years ago

In 5548ad23/git:

Repair crashes in raster aggregates, references #4916, #4770, #4724, #4916

Note: See TracTickets for help on using tickets.