Duplicated DROP AGGREGATE IF EXISTS st_samealignment(raster) in rtpostgis_upgrade_20_21.sql

[strk]

I found a duplicated DROP AGGREGATE IF EXISTS st_samealignment(rater) in rtpostgis_upgrade_20_21.sql

Dunno if it's normal.

[strk]

The first one comes from rtpostgis_drop.sql The second one is generated by rtpostgis_upgrade.sql I think the first one should not be there (ie:remove from rtpostgis_drop.sql).

Is there any reason to keep it there?

[dustymugs]

I don't believe so. Feel free to remove assuming uninstall of PostGIS is clean.

[strk]

uhm… how can we test uninstall ? That's part of "make check" isn't it ?

[strk]

I see "uninstall_rtpostgis.sql" is generated vis "create_undef.pl". Not sure what the role of rtpostgis_drop.sql.in is, given we have an "rtpostgis_upgrade_cleanup.sq" as well.

Maybe we should drop the whole rtpostgis_drop.sql.in file?

[dustymugs]

Yes, "uninstall" is part of "make check".

I forget the combinations that the upgrade/drop sql.in scripts are used to generate the output uninstall and upgrade scripts. I haven't had time to see your current upgrade script generator…

[strk]

It looksl ike rtpostgis_upgrade_cleanup.sql and rtpostgis_drop.sql are always used togheter and only to produce rtpostgis_upgrade_20_21.sql and are serialized one after the other, in that order.

REF: ​https://github.com/postgis/postgis/blob/2.1.3/raster/rt_pg/Makefile.in#L118-L119

The comment on top of rtpostgis_drop.sql.in is bogus in that the file is not used for uninstall: ​https://github.com/postgis/postgis/blob/2.1.3/raster/rt_pg/rtpostgis_drop.sql.in#L32 ​https://github.com/postgis/postgis/blob/2.1.3/raster/rt_pg/Makefile.in#L124-L125

[strk]

r12594 makes the roles of rtpostgis_upgrade_cleanup.sql and rtpostgis_drop.sql clarer, moves many items from _drop to _cleanup and removes the drop for signature-changing aggregates (taken care of by proc_upgrade).

As part of the change I also moved the rtpostgis_drop.sql _after_ the upgrade, so to catch eventual problems. This part may be dangerous. Needs more testing for extension upgrades.

[strk]

r12595 fixes accidental drop of another non-obsoleted aggregate: ST_Union(raster)

[strk]

r12598 fixes the raster_columns update and passes make check with upgrade path 2.0.5—2.1.4dev Considering this closed.

[strk]

Actually, reopening as this work would now need to be forward-ported to trunk (ouch!). Dustymugs, can you help with that, what do you think ?

The idea is to separate objects that are _required_ to be removed before the upgrade from those that can be run after. Any fully obsoleted object name can be removed after, while objects having the same name but possibly different signature would need to be handled before.

Handling both before would not hurt (as we have been doing so far) but I hope one day to have the "handled-before" ones automated via self-contained documentation about which version changed which object signature incompatibly…

[strk]

It looks like a cleanup of the drop.sql to remove non-obsoleted signature was already done in trunk with r12254