Opened 17 months ago

Last modified 6 months ago

#4386 reopened defect

[upgrade] ERROR: cannot drop function st_union(geometry) because other objects depend on it

Reported by: strk Owned by: komzpa
Priority: blocker Milestone: PostGIS 3.0.0
Component: build/upgrade/install Version: master
Keywords: Cc:

Description

Upgrades from 2.5 or in-dev are broken due to change in ST_Union signature.

See https://dronie.osgeo.org/postgis/postgis/241/3/2 which is building a PR adding ad-hoc testing for this case: https://git.osgeo.org/gitea/postgis/postgis/pulls/30

The signature of aggregate was changed by Komzpa to raise limit of 2G for usage: https://trac.osgeo.org/postgis/ticket/4340

Suggestions were provided to avoid this upgrade failure in the corresponding PR: https://github.com/postgis/postgis/pull/394

Change History (21)

comment:1 Changed 17 months ago by komzpa

Thanks for the testcase.

comment:2 Changed 17 months ago by strk

NOTE: #4352 would fix this case when running against PostgreSQL 12 or higher (just for the record, we still want to support older versions)

comment:3 Changed 16 months ago by komzpa

PR https://github.com/postgis/postgis/pull/403 (not passing tests currently)

comment:4 Changed 12 months ago by pramsey

What is the status here? I can upgrade from 2.5.

comment:5 Changed 12 months ago by pramsey

Resolution: wontfix
Status: assignedclosed

Not that this is a good thing, but we mostly have to just wait for the world to evolve to newer Pg versions that let us upgrade more transparently.

comment:6 Changed 12 months ago by strk

we mostly have to just wait for the world to evolve to newer Pg versions

My point is we could also be nice and while waiting for the world to evolve we could avoid changing names of things without a complelling reason....

It's just some cost/benefit math.

comment:7 Changed 12 months ago by dbaston

Yeah, the cost/benefit of this seems off. We break upgrade for users with ST_Union in a view, and in return we get ... possible union of inputs larger 1 gb, or not, depending on the geometry?

comment:8 Changed 12 months ago by pramsey

OK, how do we feel about waiting for a fix? Maybe a day or two to write it, and then lots of testing.

comment:9 Changed 12 months ago by pramsey

Untested, but in a working state for simple cases as far as I can tell, https://github.com/postgis/postgis/pull/489

comment:10 Changed 12 months ago by pramsey

Returned to old aggregate signature, but with new computation path for ST_Union(), other aggregates use the old path via geometry[]. Applied to trunk at r17856.

comment:11 Changed 12 months ago by Algunenano

Resolution: wontfix
Status: closedreopened

Multiple bots are complaining about this:

13:34:23 ERROR:  cannot drop function st_union(geometry) because other objects depend on it
13:34:23 DETAIL:  view upgrade_view_test depends on function st_union(geometry)

comment:12 Changed 12 months ago by pramsey

Yeah, this is #4523. It's not my fix here, it's the upgrade test PR from strk that I added along the way. It tests that ST_Union views are durable across upgrades, which I'm pretty sure they never were, because we always did drop/create on them. We only get away from that in Pg 12 which can do a create-or-replace. I'm commenting out strk's test.

comment:13 Changed 12 months ago by pramsey

In 17882:

Remove view testing from upgrade objects testing
References #4386

comment:14 Changed 12 months ago by pramsey

Resolution: fixed
Status: reopenedclosed

comment:15 Changed 8 months ago by strk

Resolution: fixed
Status: closedreopened

Why was this ticket closed ? The bug is NOT fixed, only the test was disabled ...

comment:16 Changed 7 months ago by pramsey

It was closed because the test wasn't exercising the new, bad behaviour, but a long-standing, "shrug" behaviour, which is that if you have a view based on ST_Union the upgrade process that does drop/create on aggregate functions will fail on you.

comment:17 Changed 7 months ago by Paul Ramsey <pramsey@…>

Resolution: fixed
Status: reopenedclosed

In ff6855b/git:

re-enable aggregate/view upgrade test, for better or worse, closes #4386

comment:18 Changed 7 months ago by strk

Resolution: fixed
Status: closedreopened

The test does NOT close this ticket, it just makes it tested. See https://dronie.osgeo.org/postgis/postgis/1098/1/2

Example broken upgrade path: 2.1.9--3.1.0dev

comment:19 Changed 7 months ago by strk

Being a long-standing bug, I guess we could _change_ the test to _only_ create an ST_Union based view IFF coming from PostGIS 2.5 or higher. How would you like that ?

Or, we find a way to change aggregate definitions by touching PG catalogs (when NOT using PostgreSQL 12, which supports CREATE OR REPLACE)

comment:20 Changed 6 months ago by Sandro Santilli <strk@…>

In 93bb425/git:

Do not test upgrades with of postgis backed views on PG<12

See #4386

comment:21 Changed 6 months ago by strk

I disabled the test with PG<12 for now. A smarter switch would be to disable IFF the upgrade path crosses a CHANGED notice

Note: See TracTickets for help on using tickets.