Opened 7 years ago

Closed 7 years ago

#1746 closed defect (fixed)

Upgrade scripts are not installing all functions

Reported by: robe Owned by: strk
Priority: high Milestone: PostGIS 2.0.0
Component: build/upgrade/install Version: trunk
Keywords: Cc:

Description

I haven't bothered ticketing this because a long time ago it wasn't possible when I first tried it.

If I start with a beta install and upgrade to say rc2 and then I move my postgis extension to a new schema. There are two functions left behind.

st_dump(postgis.geom), st_isvaliddetail(postgis.geometry)

Steps to create.

-- install beta2 or--
 CREATE EXTENSION postgis ;

-- install rc1 or rc2 --
ALTER EXTENSION postgis UPDATE TO "2.0.0rc2";
CREATE SCHEMA postgis;
ALTER EXTENSION postgis SET SCHEMA postgis;

When I upgrade one that I've been upgrading for a while I actually have 6 functions left behind in public.

st_dump, st_intersection(postgis.raster....) (4 permutations of this), st_isvaliddetail(postgis.geometry).

These might be signature changes we didn't drop so may be a sign of us not populating the drop scripts for these.

Change History (8)

comment:2 Changed 7 years ago by robe

Forgot to say that if I do a fresh

CREATE EXTENSION postgis;

ALTER EXTENSTION to schema.. postgis

All moves.

comment:3 Changed 7 years ago by robe

Component: postgisbuild/upgrade/install
Milestone: PostGIS 2.0.1PostGIS 2.0.0
Owner: changed from pramsey to strk
Priority: mediumhigh
Summary: Moving postgis extension to another schema leaves things behindUpgrade scripts are not installing all functions

I have determined this not to be a flaw in the extensions machinery but a flaw in our upgrade scripts that not only affects extensions but all our upgrade scripts. The issue is our script that generates the upgrade scripts is missing installing functions. It seems it misses functions if they start with a function already present with similar arguments.

For example

st_dump -- doesn't get installed presumably because our upgrade confuses it with st_dumppoints
st_isvaliddetail -- another -- looks very similar to st_isvaliddetail that takes two args 

st_intersection raster is another - this one however may be permutations of st_intersection that we aren't dropping.  I'll have to investigate that closer

This is just something that happens to be much easier to detect with extensions because extensions allow you to move all objects packaged in an extension to another schema with a 1 line command. What happens how the extension upgrade script is catching this is

1) As part of upgrade, I drop all functions from the extensions with the assumption that our upgrade scripts will reinstall all functions except possibly aggregates.

2) It doesn't apparently so the functions it doesn't install are still installed but are no longer part of the postgis extension.

It's not super super critical because I it will only affect people upgrading from very very early on in the cycle before we introduced these and will get fixed as soon as we fix this.

comment:4 Changed 7 years ago by robe

Actually add ST_IsValid to that list. Our upgrade script is generating

ST_IsValid(geometry,int)

but not ST_IsValid(geometry)

Which I'm not sure the reason why we didn't make this use default args but doesn't perhaps because it is all implemented in c.

comment:5 Changed 7 years ago by robe

Ignore the st_intersection raster ones. Those are obsolete functions that weren't being dropped and I only see them in a client that upgraded from an earlier install of postgis 2.0. I'll add to raster as things to drop.

comment:6 Changed 7 years ago by robe

Milestone: PostGIS 2.0.0PostGIS 2.0.1

I'm going to punt this to 2.0.1. I'm not willing to go thru another rc just for this.

This would only affect people who upgraded from an earlier release (that might be missing these missing functions) or those who upgraded from an earlier release and then wanted to move postgis to another schema.

I say we just put a warning in the release notes. I suspect this issue exists in 1.5 upgrade scripts but you'd never see it because all the functions it neglects to add have existed in prior versions.

comment:7 Changed 7 years ago by strk

Milestone: PostGIS 2.0.1PostGIS 2.0.0
Status: newassigned

postgis_proc_upgrade.pl expects CREATE TYPE to spawn multiple lines. The only two cases in which this doesn't happen result in eating the next item (which is a function in these cases).

comment:8 Changed 7 years ago by strk

Resolution: fixed
Status: assignedclosed

Fixed by r9592 -- it makes me nervous that we don't have an automated test for "did we upgrade _all_ functions ? did we install anything more ?"

Note: See TracTickets for help on using tickets.