Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#3892 closed task (fixed)

Review safety and behavior of CREATE EXTENSION from UNPACKAGED

Reported by: strk Owned by: pramsey
Priority: high Milestone: PostGIS 2.4.2
Component: postgis Version: 2.4.x
Keywords: Cc: robe

Description

It isn't clear to me if it is legal to CREATE EXTENSION postgis FROM unpackaged and what happens if the unpackaged version is different from the target version. Is it documented anywhere ?

If we're only guaranteeing for things to work when versions match we should add a safety check to make sure that versions do indeed matc.

Change History (13)

comment:1 by robe, 7 years ago

strk,

I did a test and though it's unorthodox, you can move a function from one extension to another with a unpackaged.

So I created a blackhole extension:

— blackhole.control looks like this

comment = 'Black Hole'
default_version = '1.0'
relocatable = true

— blackhole—unpackaged—1.0.sql

CREATE OR REPLACE FUNCTION do_something() RETURNS text AS
$$
SELECT 'test';
$$ language sql;

ALTER EXTENSION postgis DROP FUNCTION _add_raster_constraint_blocksize(
    rastschema name,
    rasttable name,
    rastcolumn name,
    axis text);
    
ALTER EXTENSION blackhole ADD FUNCTION  _add_raster_constraint_blocksize(
    rastschema name,
    rasttable name,
    rastcolumn name,
    axis text);

Then I did

CREATE EXTENSION postgis;
CREATE EXTENSION blackhole FROM unpackaged;
DROP EXTENSION postgis;

And all that was left was the function I moved into the black hole and the do_something function defined in the black hole.

So I guess as long as you stuff the raster upgrade script in the unpackaged script you can do it.

So the instruction would be, if you use raster and want to keep it, first run

CREATE EXTENSION postgis_raster FROM unpackaged;
ALTER EXTENSION postgis UPDATE;

The ALTER EXTENSION postgis would remove all raster functions/type (both drop the function and remove from postgis). It will gracefully fail if the user has raster and your warning would be, to fix

Do the above.

comment:2 by strk, 7 years ago

Sorry Regina but this ticket is about reviewing the safety for users doing "CREATE EXTENSION from unpackaged".

What an "unpackaged" upgrade script can or cannot do is about a different topic. I'm concerned about whether or not "CREATE EXTENSION <postgis*> FROM unpackaged" is safe and how to make sure it becomes safe if it is not.

comment:3 by robe, 7 years ago

Ah that sorry I misunderstood the question.

It assumes the version you have in your database is what you installed. For most extensions that generally works because they rarely add new functions and their functions are all c-based (no sql). For us not so much.

People would have to know what version of postgis they installed and do something like

CREATE EXTENSION postgis FROM unpackaged VERSION "2.4.0";
ALTER EXTENSION postgis UPDATE;

So it would run the version 2.4.0 script instead of the default 2.5.0 script.

or you'd put in a guard in the unpackaged script to check (so you'll still need to install one of our temporary helper functions) and error out if what postgis_full_version tells you doesn't match with the version of the unpackaged script.

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

comment:4 by strk, 7 years ago

Regina: what if you loaded postgis-2.2.0 unpackaged and then install postgis-2.4.0 and CREATE EXTENSION postgis VERSION 2.4.0 FROM unpackaged ?

I've tried doing 2.1 to 2.3 and got:

ERROR:  aggregate st_clusterintersecting(geometry) does not exist

That one seems like the best case (an error is raised, although obscure). What's the worst case ?

If we make a version check ourselves we get:

1) More user-friendly error message 2) Protection against worst case (yet to be defined..)

*OR* (but could be also done in a second ticket?) we do the upgrade-on-load (dangerous in case the load would effectively be a _downgrade_ for any reason)

comment:5 by robe, 7 years ago

In all cases, the unpackaged would fail if it tries to add a type, function, agg that is not installed so I think the worse case scenario of your lib pointing at 2.2 but your unpackaged stamps it as 2.4 is accounted for

Worst case is they don't have an installation of what they came from so no unpackaged script for them to run so they are stuck unpackaged because the only ones available error out.

If the unpackaged actually works, then what postgresql thinks you are running is not what you have installed.

.

For this case

SELECT postgis_full_version();

Would show the inconsistency. However postgis_full_version() doesn't tell them what to do if their postgis extension say they are at 2.4.1 but they are really at 2.4.0.

So postgis_full_version() whould need to look at the pg_available_extensions.installed_version and compare it to what postgis_lib_version() says and if it's different and pg_available_extensions.installed_version = pg_available_extensions.default_version

tell them the need to run

ALTER EXTENSION postgis UPDATE TO "2.4.1next";

ALTER EXTENSION postgis UPDATE TO "2.4.1";

comment:6 by strk, 7 years ago

Worst case is they don't have an installation of what they came from so no unpackaged script for them to run so they are stuck unpackaged because the only ones available error out.

In this case it would be nice of us to make the "FROM unpackaged" embed an upgrade, wouldn't it ?

If the unpackaged actually works, then what postgresql thinks you are running is not what you have installed.

This case would also be fixed if we make the "FROM unpackaged" embed an upgrade, right ?

So postgis_full_version() whould need to look at the pg_available_extensions.installed_version and compare it to what postgis_lib_version() says

Could you ticket this one separately, it's a good idea !

comment:7 by pramsey, 7 years ago

Milestone: PostGIS 2.4.1PostGIS 2.4.2

comment:8 by strk, 7 years ago

Regina what do you think about this: https://git.osgeo.org/gogs/postgis/postgis/pulls/19

comment:9 by robe, 7 years ago

Looks good.

comment:10 by strk, 7 years ago

Resolution: fixed
Status: newclosed

In 16046:

Check version matching upon creating extension from unpackaged

Closes #3892

comment:11 by strk, 7 years ago

Thanks for review, committed as r16046 = 563b4b671a6676d48e64e989c4f5964b608c97e6 (refs/remotes/svn/trunk)

I guess we'll want to do the same for the other extensions too

comment:12 by strk, 7 years ago

In 16048:

Fix unpackaged extension creation check

See #3892

comment:13 by strk, 7 years ago

In 16049:

Check unpackaged version for topology too

See #3892

Note: See TracTickets for help on using tickets.