2.2 -> 2.3.0dev leaves 15 function behind

[strk]

After a successful EXTENSION UPDATE to 2.3.0dev I still have 15 functions pointing to the 2.2 library:

=# select proname from pg_proc where probin = '$libdir/postgis-2.2';
               proname
-------------------------------------
 geometry_spgist_leaf_consistent_2d
 geometry_spgist_inner_consistent_2d
 geometry_spgist_picksplit_2d
 geometry_spgist_config_2d
 geometry_spgist_choose_2d
 st_astwkb
 st_seteffectivearea
 pgis_twkb_accum_transfn
 pgis_twkb_accum_transfn
 pgis_twkb_accum_transfn
 pgis_twkb_accum_transfn
 pgis_twkb_accum_finalfn
 geometry_distance_centroid
 geometry_distance_box
 geometry_distance_box_nd
(15 rows)

[strk]

It may be that these functions where only temporarily available in any development branch as my database is often upgraded during development. In any case adding them to the respective DROP files should not hurt.

[strk]

List including arguments:

=# select proname,pg_get_function_arguments(oid) from pg_proc where probin = '$libdir/postgis-2.2';
               proname               |                                  pg_get_function_arguments                                   
-------------------------------------+----------------------------------------------------------------------------------------------
 geometry_spgist_leaf_consistent_2d  | internal, geometry, integer
 geometry_spgist_inner_consistent_2d | internal, geometry, integer
 geometry_spgist_picksplit_2d        | internal, internal
 geometry_spgist_config_2d           | internal, internal
 geometry_spgist_choose_2d           | internal, internal
 st_astwkb                           | geometry, integer, bigint DEFAULT NULL::bigint, boolean DEFAULT false, boolean DEFAULT false
 st_seteffectivearea                 | geometry, double precision DEFAULT 0
 pgis_twkb_accum_transfn             | internal, geometry, integer
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint, boolean
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint, boolean, boolean
 pgis_twkb_accum_finalfn             | internal
 geometry_distance_centroid          | geom1 gidx, geom2 gidx
 geometry_distance_box               | gidx1 gidx, gidx2 gidx
 geometry_distance_box_nd            | geom1 geometry, geom2 geometry
(15 rows)

[strk]

The "st_astwkb" one was left behind by r13493 (for #3085)

[strk]

The "st_aswkb" one never saw the light in an official release because 2.2.0 already shipped the new signature.

[robe]

Just tried upgrading from 2.2.1 (released version), to 2.3.0dev. First time around I got the gdal_path redefined notice (this was using the GUI in pgAdmin).

Then trying again with :

ALTER EXTENSION postgis
  UPDATE TO "2.3.0dev";

It succeeded but gave these notices:

WARNING:  'postgis.backend' is already set and cannot be changed until you reconnect
CONTEXT:  SQL statement "SELECT                  postgis_lib_version()"
PL/pgSQL function postgis_major_version_check() line 21 at SQL statement
Query returned successfully with no result in 350 msec.

Then when I dropped the extension and attempted to recreate with

create extension postgis version "2.2.1";

I got failure -

ERROR:  attempt to redefine parameter "postgis.gdal_datapath"

This is

PostgreSQL 9.4.4, compiled by Visual C++ build 1800, 64-bit

So it does seem we've still got problems.

[strk]

(In [14629]) Unregister all functions before starting postgis extention upgrade

Also drop temporarely introduced ST_AsTWKB signature. See #3450

[strk]

Robe please use another ticket for the gdal_datapath issue. This one is to not leave dropped function behind. Left over after r14629 (still to be backported to 2.2 branch) are:

geometry_spgist_leaf_consistent_2d  | internal, geometry, integer
 geometry_spgist_inner_consistent_2d | internal, geometry, integer
 geometry_spgist_picksplit_2d        | internal, internal
 geometry_spgist_config_2d           | internal, internal
 geometry_spgist_choose_2d           | internal, internal
 st_seteffectivearea                 | geometry, double precision DEFAULT 0
 pgis_twkb_accum_transfn             | internal, geometry, integer
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint, boolean
 pgis_twkb_accum_transfn             | internal, geometry, integer, bigint, boolean, boolean
 pgis_twkb_accum_finalfn             | internal
 geometry_distance_centroid          | geom1 gidx, geom2 gidx
 geometry_distance_box               | gidx1 gidx, gidx2 gidx
 geometry_distance_box_nd            | geom1 geometry, geom2 geometry

[strk]

(In [14630]) Revert "Unregister all functions before starting postgis extention upgrade"

This reverts r14629 (see #3450)

The code was broken (but the idea is still good to evaluate)

[strk]

Code was broken, r14630 reverts it. All functions are left-over at the moment.

I guess a general solution would be to intercept and wrap every line starting with DROP to make it extension-aware, and I even see it done for the "rtpostgis_upgrade.sql" rule.

But I won't be looking at this any soon, so if you want to continue here Regina, you're welcome!

[strk]

I'm actually looking at this now. Regina, is the comment about "CREATE|DROP CAST" in extension/postgis/Makefile.in still valid ? It says postgis_extension_drop_if_exists cannot drop CASTs but then there's code to produce rtpostgis_upgrade.sql in the _same_ Makefile.in that does use it ?

[strk]

(In [14631]) Wrap every DROP to first unregister object from postgis extension

Also drop temporarely introduced ST_AsTWKB signature. See #3450

[strk]

Regina, see how you like r14631. I'd also remove the CAST lines there, to further simplify things.

[strk]

(In [14632]) Further simplify extension wrapping

See #3450

[strk]

r14632 also removes special handling of CAST and threats the same postgis and raster upgrades. I've tested this with:

cd regress
run_test.pl --extension --upgrade --upgrade-path 2.2.1--2.3.0dev tickets.sql

Which reports no problem.

[strk]

(In [14633]) Unregister all functions before starting postgis extention upgrade

Also drop temporarely introduced ST_AsTWKB signature. See #3450

[strk]

(In [14634]) Wrap every DROP to first unregister object from postgis extension

Also drop temporarely introduced ST_AsTWKB signature. See #3450

[strk]

Backported to 2.2 branch with r14633 (finally simplified version)

[strk]

(In [14636]) Fix mistakenly committed block (see #3450)

[strk]

Actually r14634 is the good 2.2 backport, r14633 was a mistake fixed with r14636

[strk]

The pgis_twkb_accum_transfn and pgis_twkb_accum_finalfn functions were first added with r11736 (2.2.0dev) they never saw the light in 2.2.0 final.

[strk]

I tried dropping the pgis_twkb_accum_* functions but found that the "postgis_extension_drop_if_exists" function silently fails:

strk=# SELECT postgis_extension_drop_if_exists('postgis', 'DROP FUNCTION IF EXISTS pgis_twkb_accum_finalfn(internal)');
 postgis_extension_drop_if_exists 
----------------------------------
 t
(1 row)
strk=# DROP FUNCTION IF EXISTS pgis_twkb_accum_finalfn(internal); -- temporarely introduced before 2.2.0 final
ERROR:  cannot drop function pgis_twkb_accum_finalfn(internal) because other objects depend on it
DETAIL:  extension postgis depends on function pgis_twkb_accum_finalfn(internal)

[strk]

The problem is not with the postgis_extension_drop_if_exists function but with ALTER EXTENSION itself !

strk=# alter extension postgis DROP FUNCTION pgis_twkb_accum_finalfn(internal);
ALTER EXTENSION
strk=# DROP FUNCTION pgis_twkb_accum_finalfn(internal);
ERROR:  cannot drop function pgis_twkb_accum_finalfn(internal) because other objects depend on it
DETAIL:  extension postgis depends on function pgis_twkb_accum_finalfn(internal)
strk=# select version();
                                               version                                                
------------------------------------------------------------------------------------------------------
 PostgreSQL 9.3.6 on x86_64-unknown-linux-gnu, compiled by gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2, 64-bit
(1 row)

David maybe this is one of those things that would help if done upstream ...

[strk]

PostgreSQL itself confirms that the function is unregistered from the postgis extension:

-- Drop it from extension
strk=# begin; alter extension postgis DROP FUNCTION pgis_twkb_accum_finalfn(internal);
BEGIN  
ALTER EXTENSION

-- Drop it again from extension, showing that the removal was successful
strk=# alter extension postgis DROP FUNCTION pgis_twkb_accum_finalfn(internal);
ERROR:  function pgis_twkb_accum_finalfn(internal) is not a member of extension "postgis"

-- Rollback, drop it from the extension (now we know it's successful)
strk=# rollback; begin; alter extension postgis DROP FUNCTION pgis_twkb_accum_finalfn(internal);
ROLLBACK
BEGIN  
ALTER EXTENSION

-- Drop the function, and PostgreSQL complains that it still registered as part of extension !
strk=# DROP FUNCTION pgis_twkb_accum_finalfn(internal);
ERROR:  cannot drop function pgis_twkb_accum_finalfn(internal) because other objects depend on it
DETAIL:  extension postgis depends on function pgis_twkb_accum_finalfn(internal)

[strk]

Figured: the function is actually a dependency of an aggregate, which is NOT removed at the time of DROP FUNCTION. So the only issue with PostgreSQL is that the error message is confusing, not giving the direct dependent of the function

[strk]

(In [14638]) Drop temporarely added tkwb aggregate and support functions

These were added during 2.2.0 development but removed before going final.

See #3450

[strk]

(In [14639]) Drop temporarely added tkwb aggregate and support functions

These were added during 2.2.0 development but removed before going final.

See #3450

[strk]

r14638 removes the TWKB aggregate and support function in trunk (2.3.0dev) -- r14639 in 2.2 branch (for 2.2.2)

[strk]

I made a mess with the 2.2.0 tag, pushing there rather than in 2.2 branch the changes. The whole mess should be resolved with r14640 and r14641 so that r14641 brings the "2.2.0" code back to where it was with r14208.

Really sorry for the confusion, now I'll have to backport again the changes to the real 2.2 branch.

Reminder to self: "git svn info" helps as well as "git svn dcommit --dry-run"

[strk]

(In [14642]) Wrap every DROP to first unregister object from postgis extension

Also drop temporarely introduced ST_AsTWKB signature. See #3450

[strk]

(In [14643]) Drop temporarely added tkwb aggregate and functions

These were added during 2.2.0 development but removed before going final.

See #3450

[strk]

So r14642 is the 2.2(2dev) backport of extension drop machinery, r14643 the one dropping TWKB temporary functions and aggregates

[strk]

Left-over functions:

strk=# select proname,pg_get_function_arguments(oid) from pg_proc where probin = '$libdir/postgis-2.2';
               proname               |      pg_get_function_arguments       
-------------------------------------+--------------------------------------
 geometry_spgist_leaf_consistent_2d  | internal, geometry, integer
 geometry_spgist_inner_consistent_2d | internal, geometry, integer
 geometry_spgist_picksplit_2d        | internal, internal
 geometry_spgist_config_2d           | internal, internal
 geometry_spgist_choose_2d           | internal, internal
 st_seteffectivearea                 | geometry, double precision DEFAULT 0
 geometry_distance_centroid          | geom1 gidx, geom2 gidx
 geometry_distance_box               | gidx1 gidx, gidx2 gidx
 geometry_distance_box_nd            | geom1 geometry, geom2 geometry
(9 rows)

[strk]

I cannot find any reference to "geometry_spgist" in the repository history, so I guess it was in the GSoC SPGIST drive. I will not drop code that was never added in an official PostGIS branch.

[strk]

Confirmed, spgist comes from here: ​https://github.com/mohitkharb/postgis/tree/spgist

[strk]

Left over, after manually dropping the spgist ones:

strk=# select proname,pg_get_function_arguments(oid) from pg_proc where probin = '$libdir/postgis-2.2';
          proname           |      pg_get_function_arguments       
----------------------------+--------------------------------------
 st_seteffectivearea        | geometry, double precision DEFAULT 0
 geometry_distance_centroid | geom1 gidx, geom2 gidx
 geometry_distance_box      | gidx1 gidx, gidx2 gidx
 geometry_distance_box_nd   | geom1 geometry, geom2 geometry
(4 rows)

ST_SetEffectiveArea was first added in r13174 by Nicklas Avén who later changed its signature in r13418 w/out adding a DROP.

[strk]

(In [14644]) Drop temporarely existing deprecated st_effectivearea signature

See #3450

[strk]

(In [14645]) Drop temporarely existing deprecated st_effectivearea signature

See #3450

[strk]

st_seteffectivearea drop added with r14645 in 2.2 branch and r14644 in trunk. I cannot find any referenc eto "geometry_distance.*gidx" in history, while the "geometry_distance_box_nd(geometry,geometry)" function entered the repository with r13287 (mine) and was removed by r13607 (Paul's). It's another case of pre 2.2.0, to be removed.

[strk]

(In [14646]) Drop temporarely added geometry_distance_box_nd(geometry,geometry)

See #3450

[strk]

(In [14647]) Drop temporarely added geometry_distance_box_nd(geometry,geometry)

Closes #3450