Opened 10 years ago

Closed 10 years ago

#2750 closed defect (fixed)

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

Reported by: strk Owned by: Bborie Park
Priority: medium Milestone: PostGIS 2.1.4
Component: raster Version: 2.1.x
Keywords: Cc:

Description

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

Dunno if it's normal.

Change History (12)

comment:1 by strk, 10 years ago

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?

comment:2 by Bborie Park, 10 years ago

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

comment:3 by strk, 10 years ago

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

comment:4 by strk, 10 years ago

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?

comment:5 by Bborie Park, 10 years ago

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…

comment:6 by strk, 10 years ago

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

comment:7 by robe, 10 years ago

Component: postgisraster
Owner: changed from pramsey to Bborie Park

comment:8 by strk, 10 years ago

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.

comment:9 by strk, 10 years ago

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

comment:10 by strk, 10 years ago

Milestone: PostGIS 2.1.4
Resolution: fixed
Status: newclosed

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

comment:11 by strk, 10 years ago

Resolution: fixed
Status: closedreopened

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…

comment:12 by strk, 10 years ago

Resolution: fixed
Status: reopenedclosed

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

Note: See TracTickets for help on using tickets.