Opened 6 years ago

Closed 6 years ago

#3913 closed defect (fixed)

Include minor upgrade in extension-from-unpackaged script

Reported by: strk Owned by: robe
Priority: medium Milestone: PostGIS 2.5.0
Component: postgis Version: 2.4.x
Keywords: Cc:

Description

As discussed in https://trac.osgeo.org/postgis/ticket/3888#comment:15 it makes sense to always upgrade unpackaged scripts before wrapping in an extension, and it would make it more friendly to switch form unpackaged to packaged.

If we do this, the change introduced in #3892 would need to be reverted, of course.

Change History (15)

comment:1 by robe, 6 years ago

Milestone: PostGIS 2.5.0
Owner: changed from pramsey to robe

comment:3 by strk, 6 years ago

Resolution: fixed
Status: newclosed

In 16063:

Upgrade during create extension from unpackaged

This commit does it for "postgis" and "postgis_topology"
extensions.

Closes #3913

comment:4 by strk, 6 years ago

Resolution: fixed
Status: closedreopened

I think this commit might have introduced a conceptual error. When including minor upgrade it may happen that the minor upgrade runs a CREATE statement, so that the created new object is installed in the target extension automatically. Later on, when ALTER EXTENSION .. ADD is called if the object is already in the extension you get an error like: ERROR: view raster_columns is already a member of extension "postgis"

Regina: can you produce such occurrence ? I'm not sure our bots are triggering it (but could be wrong) — I'm getting it manually from my "split-raster-from-core" branch…

comment:5 by strk, 6 years ago

What we could do (after making sure our bots trigger the failure) is wrapping the ALTER EXTENSION ADD and catch exception after carefully checking that the exception is really about "object is already in target extension"

comment:6 by strk, 6 years ago

I confirm the problem exists in trunk too. Way to reproduce:

 createdb t
 for f in  postgis/postgis.sql raster/rt_pg/rtpostgis.sql spatial_ref_sys.sql; do 
    psql -f $f t
 done
 psql -c 'create extension postgis from unpackaged' t

comment:7 by strk, 6 years ago

The exception being thrown is a code 55000 aka "object_not_in_prerequisite_state"

comment:8 by strk, 6 years ago

Patch to catch "object_not_in_prerequisite_state" is here: https://gitlab.com/postgis/postgis/merge_requests/7

A second pair of eyes on the safety of such approach is welcome.

Of course we're still missing an automated test from bots or even "make check" for the unpackaged-to-extension loading

comment:9 by strk, 6 years ago

I've been pointed out to the fact that "object_not_in_prerequisite_state" would also be thrown if the object being added to an extension is already registered as being part of _another_ extension, and that there are possibly other cases in which it can be thrown which do not even relate to extensions…

So the code should be further modified to "manually" implement the "IF NOT EXISTS" part

comment:10 by strk, 6 years ago

In 16076:

Have output of create_unpackaged survive already-registered objects

References #3913

comment:11 by strk, 6 years ago

In 16077:

Include upgrade from unpackaged test in check_all_upgrades script

References #3913

comment:12 by strk, 6 years ago

r16078 further restricts the exception acceptance condition by requiring the associated error message says 'already included in "postgis"'. Not necessarely safe from localized installs (we lack tests for that, I'm afraid)

comment:13 by strk, 6 years ago

Resolution: fixed
Status: reopenedclosed

In 16079:

Make already-registered check in unpackaged extension create safer

This version just checks for "postgis" with word boundaries, so
it is immune to localization.

Closes #3913 again

comment:14 by strk, 6 years ago

Resolution: fixed
Status: closedreopened

This needs to be fixed for postgis_sfcgal too (see #4000).

comment:15 by strk, 6 years ago

Resolution: fixed
Status: reopenedclosed

In 16345:

Include minor upgrade in sfcgal unpackaged upgrade

Closes #3913 again (for sfcgal) and hopefully fixes #4000

Note: See TracTickets for help on using tickets.