Opened 3 years ago

Closed 3 years ago

#3650 closed defect (fixed)

Aggregate functions aren't marked as parallel safe (mark aggregates that don't use transition)

Reported by: komzpa Owned by: robe
Priority: medium Milestone: PostGIS 2.3.1
Component: postgis Version: 2.3.x
Keywords: Cc:

Description

Right now in PostGIS 2.3 aggregate functions aren't marked as parallel safe. That leads to Postgres cancelling all the possible parallelizations if there's such a function anywhere within the query.

Fix: https://github.com/postgis/postgis/pull/117

Change History (8)

comment:1 Changed 3 years ago by strk

Owner: changed from pramsey to robe

Regina, the changes look sensible from here (but I don't have a parallel-ready PostgreSQL install to check the actual consequences). Any reason not to apply them ?

comment:2 Changed 3 years ago by robe

I can do some quick queries with large datasets just to confirm no consequences and then I'll commit if it looks okay.

I think there is some funky stuff with the PostgreSQL handling of parallel aggregates (it's PostgreSQL specific though). For example when I was updating data (geocioding) , the counts of what geocoded was going up and down (when it should have been only increasing). It was using a parallel strategy. I was ignoring it for the most part, but have to revisit.

I want to double check that to see if it only happens during updates. If it is a bug, I'd rather wait and implement this when 9.6.1 comes out.

comment:3 Changed 3 years ago by robe

Summary: Aggregate functions aren't marked as parallel safeAggregate functions aren't marked as parallel safe (mark aggregates that don't use transition)

I'm not sure it's safe to mark aggregates that use transition functions as safe, so I'm leaving those out. We can maybe revisit those for 2.4.

Last edited 3 years ago by robe (previous) (diff)

comment:4 Changed 3 years ago by robe

This is annoying our uninstall script appears to require basetype in there for aggregates.

basetype is considered deprecated syntax and has been for a while. As noted in postgis thread, the safe stuff doesn't work if basetype is there. https://www.postgresql.org/message-id/b1e247ca-23ad-1d24-cecc-28600550f7bb@postgrespro.ru

presumably because basetype is arcane syntax they don't have work with newer syntax.

https://www.postgresql.org/docs/9.2/static/sql-createaggregate.html

I'm super puzzled though how this pull request ever made it thru our regression.

When I run I get:

make[1]: Entering directory '/projects/postgis/branches/2.4/postgis'
/bin/perl ../utils/create_undef.pl postgis.sql 95 > uninstall_postgis.sql
Couldn't parse AGGREGATE line: CREATE AGGREGATE ST_Extent(
        sfunc = ST_CombineBBox,
        finalfunc = box2d,
        stype = box3d
        );

even when against lower than 9.6, presumably because I removed the basetype and our perl script

/create aggregate\s*([\w\.]+)\s*\(\s*.*basetype = ([\w\.]+)/ism )

I'm guessing the reason I am triggering this error is because I put in a

-- Changed: 2.3.1 to support PostgreSQL 9.6 parallel safe

triggering the uninstall script to actually write in a drop and recreate aggregate.

Last edited 3 years ago by robe (previous) (diff)

comment:5 Changed 3 years ago by strk

It sounds like the aggregate syntax thing needs a separate ticket. Please specify which version of PostgreSQL supports the new syntax.

comment:6 Changed 3 years ago by robe

It might not. Just realize a typo in my fix.

It has to be ST_Extent(geometry)() and I still had ST_Extent(...). so I'll see and change if its necessary.

comment:7 Changed 3 years ago by robe

In 15255:

Mark ST_Extent, ST_Mem* aggregate functions parallel safe
references #3650 for PostGIS 2.4 (trunk)

comment:8 Changed 3 years ago by robe

Resolution: fixed
Status: newclosed

In 15256:

Mark ST_Extent, ST_Mem* aggregate functions parallel safe
closes #3650 for PostGIS 2.3.1

Note: See TracTickets for help on using tickets.