Split core and raster extensions

[strk]

This is a proposal to separate the core of postgis from its raster support when using extensions.

A pull request is ready for review here: ​https://git.osgeo.org/gogs/postgis/postgis/pulls/18

It introduces a "postgis_raster" extension, depending on "postgis" and handle upgrades by turning raster functions into an unpackaged set, so they can be packaged again via "CREATE EXTENSION postgis_raster FROM unpackaged" (or dropped via rtraster_uninstall.sql).

Comments welcome.

BTW: we need an extension component

[strk]

See #3859, we need to make sure —without-raster is still able to produce the postgis extension upgrade script (as it now depends on raster uninstall scripts)

[robe]

-1 the more I think about it, the more I am against splitting postgis_raster from postgis. I'm +1 on allowing users who have —witout-raster have an extension build as long as the version name is different, something like 2.5.0noraster.

The reason is

1. Raster would be the only type not in postgis extension that has index bindings that piggy back on PostGIS geometry and also has GUCs stamped with postgis. and all it's postgis geometry function calls also have @extschema@ qualified which presumes it is installed in same schema as postgis.

Having it as a separate extension could conceivably allow people to install it in a separate schema other than where they installed postgis, which would make the extension unusable.

2. All your proposals thus far break-backward compatibility in a painful way.

Like if a user is restoring a raster backup from 2.4.0, how are you going to deal with that. Your idea of unpackaging and leaving it up to the user to repackage is very disturbing.

3. I like having my raster in postgis I use it and why are you making me do more work just to satisfy your flawed sense of beauty. Having postgis_raster as a separate extension is not beautiful to me. It just makes my life harder and the life of everyone that uses raster harder.

4. With your proposal I have do do

ALTER EXTENSION postgis UPDATE;
ALTER EXTENSION postgis_raster UPDATE;

If people really don't want raster, they should compile —without-raster

and they should get a postgis version that says 2.5.0noraster.

If people want some databases to have raster and some not, then they should do

CREATE EXTENSION postgis VERSION "2.5.0noraster";

If they then decide to add raster and they have the full-features version of postgis, they would do

ALTER EXTENSION postgis VERSION "2.5.0noraster";

users who just have —without raster can do

ALTER EXTENSION postgis UPDATE;

5. strk you don't even like extensions, why do you feel the need to muck with them.

You thought it was a stupid idea for me to introduce extension support in PostGIS in the first place. People who don't like extensions have always had the option to install the postgis scripts and raster scripts separately.

6. The other nice thing about this, is going this way, it's much easier to fix in 3.0 when PostgreSQL extension machinery will be improved and it doesn't break backward compatibility now. What you are asking for to me should really be done 3.0 if we think it's necessary.

To solve the immediate need of having extension without-raster, as I have said time and time again, the easiest way (that doesn't break backward-compatibility) is to encode it in the extension version.

To me encoding it in the extension version is the same as saying Ubuntu Server 11.04 vs. Ubuntu Desktop 11.04. They are essentially the same thing but one has a head and one does not.

[strk]

Having it as a separate extension could conceivably allow people to install it in a separate schema other than where they installed postgis, which would make the extension unusable.

We can add a safety check. This is already a problem for non-extension users, and we want to threat every user equally here. Could you ticket this for non-extension users of raster please ?

Like if a user is restoring a raster backup from 2.4.0, how are you going to deal with that. Your idea of unpackaging and leaving it up to the user to repackage is very disturbing.

We cannot repackage ourselves from a script upgrade, and we don't really want either or our postgis upgrade scripts would depend on availability of raster extension. But we print a warning recommending to call the missing "CREATE EXTENSION postgis_raster FROM unpackaged" to compete the upgrade (I like your idea of NOT proposing that IFF the raster objects can be completely removed).

With your proposal I have do do

ALTER EXTENSION postgis UPDATE; ALTER EXTENSION postgis_raster UPDATE;

Yes, if you need both. It's the same you have to do with postgis_topology and address_standardizer etc. Consistent.

If people really don't want raster, they should compile —without-raster

Not everyone can compile its own PostGIS.

As for your other proposal (encoding in version) please file a separate ticket so we can discuss it in depth.

[robe]

Yes, if you need both. It's the same you have to do with postgis_topology and address_standardizer etc. Consistent.

It's not consistent as I mentioned. First of all address_standardizer doesn't even need postgis at all.

postgis_topology is in it's own schema so can never be part of the postgis extension. Besides postgis_topology is not as intimately tied to postgis as raster is. It has no support for indexes and doesn't have GUC hooks.

If people really don't want raster, they should compile —without-raster

Not everyone can compile its own PostGIS.

Exactly and my proposal takes care of those people as well. The only inconvenience for them is having to know about the new "2.5.0noraster" while other users don't need to know anymore than what they do now.

CREATE EXTENSION postgis;

That is not an inconvenience because this is a new feature we are offering them, the feature of being able to have less PostGIS. So they should be learning a new command for a new feature.

As for your other proposal (encoding in version) please file a separate ticket so we can discuss it in depth.

Done at #3890

[robe]

strk BTW regardless how we go most of your work is still useful. To implement what I detailed in #3890 the extra bonus feature, we'll need a clean unintall_rtpostgis.sql script to include in the script that takes you from 2.5.0—2.5.0norster.

[strk]

postgis_topology is not as intimately tied to postgis as raster is.

Can you tell more about this intimate ties ? They are not clear to me. Should we discuss on the list, btw, rather than here ?

[robe]

postgis_topology is not as intimately tied to postgis as raster is.

Can you tell more about this intimate ties ? They are not clear to me. Should we discuss on the list, btw, rather than here ?

1) postgis raster has GUCS that affect it's behavior - ​http://postgis.net/docs/manual-2.4/reference.html#PostGIS_GUC it would seem a bit odd to register gucs in postgis for a separate extesnion. postgis_sfcgal actually has a similar issue but not quite since it's gucs are to swap out postgis same named functions with its own. So utiltizing the backend doesn't actually require postgis_sfcgal to be installed to use the feature.

2) postgis rasters whole index framework is built on top of postgis geometry. Granted this doesn't affect install and deinstall, but it's intimate to me.

Sure let's propose both options on mailing list, but I'm pretty tired of repeating myself and granted it does take a very deep level of understanding of how extension glue is setup, I suspect people will just say postgis_raster as a separate extension, that seems okay to me without thinking about the deep ramifications this has.

[strk]

1) postgis raster has GUCS that affect it's behavior - ​http://postgis.net/docs/manual-2.4/reference.html#PostGIS_GUC it would seem a bit odd to register gucs in postgis for a separate extesnion.

I don't see this as being an issue. It's just a namespace.

2) postgis rasters whole index framework is built on top of postgis geometry. Granted this doesn't affect install and deinstall, but it's intimate to me.

It's a dependency. PostGIS Raster *depends_on* PostGIS Core. We can and do encode the dependency, you cannot enable raster support w/out also enabling core.

[giohappy]

I don't have a strong opinion on this, and you have way more experience then me on deployment scenarios. I can only report a positive feedback regarding ​https://git.osgeo.org/gogs/postgis/postgis/pulls/18. I've jsut tested it, from compile to CREATE EXTENSION. Raster isn't there and the core PostGIS is working as expected.

I don't have statistics on common deployments. I suppose that raster opt-in vs raster opt-out should depend on the average usage: do most of users prefer to have raster by default and opt-out eventually, or the opposite?

[strk]

It is not a matter of default but possibility to do opt-in or opt-out. Right now there's no opt-out possiblity while using extensions.

[giohappy]

I mean that by splitting the extensions like you did is kind of an opt-in, because CREATE EXTENSION postgis wouldn't bring raster in. While Regina's suggestion is to have a specific "noraster" extension, and having CREATE EXTENSION postgis bringing raster in by default, and it's knod of an opt-out setup.

[strk]

With Regina's proposal the default is not determined by PostGIS itself, but by whoever installs it. What you get on CREATE EXTENSION postgis depends on what's written in postgis.control.

I guess we could force it to be an opt-out by always setting the default to the raster flavor, which would mean CREATE EXTENSION postgis would error out if you don't have raster support …

[strk]

What needs to be tested are upgrade paths. Can you help with that Giohappy ?

[strk]

Regina: I suspect the ALTER EXTENSION DROP lines in extension upgrade are not working, so raster is NOT really being unpackaged. This would leave us as still unable to do the unpackaging.

Maybe postgis_extension_unpackage() and postgis_raster_extension_unpackage() functions would help in this reguard, but even better it would be for PostgreSQL to support unpackaging natively…

[strk]

I gave this another look and actally think that ALTER EXTENSION DROP in extension upgrade are working fine. But the current obstacle is that unpackaging raster objects make them impossible to re-package w/out first upgrading them, which means having to resort to scripts loading.

So the only way I can see to solve this in an "extension way" would be for "postgis_raster—unpackaged—TARGET.sql" to include a minor upgrade. I dunno why we shouldn't be doing this always, for all extensions.

[robe]

fine with me to always do this. As I mentioned on IRC, I always hack the included unpackaged scripts for PostgreSQL contrib extensions I use to include an upgrade step. Seems to work fine.

[strk]

I've ticketed including upgrade in unpackaged script in #3913

[strk]

Merge request moved to ​https://gitlab.com/postgis/postgis/merge_requests/6 And rebase to trunk after inclusion of upgradein unpackaged landed in trunk. Also, I'v eticketed postgis_extension_unpackaged() as #3918

[strk]

The gitea PR (which includes upgrade tests) is green: ​https://git.osgeo.org/gitea/postgis/postgis/pulls/20 GitLab PR is also green: ​https://gitlab.com/postgis/postgis/merge_requests/6

Current state of branch is that on upgrade you get raster unpackaged and a WARNING message telling you to either repackage (if needed) or drop (if unneeded):

test=# alter extension postgis update;
WARNING:  unpackaging raster
WARNING:  PostGIS Raster functionality have been unpackaged
HINT:  type `CREATE EXTENSION postgis_raster FROM unpackaged` to re-package or source `uninstall_rtpostgis.sql` to drop.
WARNING:  'postgis.backend' is already set and cannot be changed until you reconnect
ALTER EXTENSION

Regina suggested to directly drop the unpackaged raster but I'm not sure how we can tell if that support is still needed by the caller.

[strk]

With r16719 it should now be safe to source unload_rtpostgis.sql in presence of columns with the 'raster' type (it would still be cascading to functions using the 'raster' type as an argument or return type.

[strk]

In 16756:

Add raster split change in NEWS

Closes #3888

[strk]

FYI: the whole set of changes are the sequencial revisions from r16730 to r16756