Opened 7 months ago

Closed 7 months ago

#5046 closed defect (fixed)

Old postgis library needed to upgrade

Reported by: robe Owned by: strk
Priority: medium Milestone: PostGIS 3.0.5
Component: build/upgrade/install Version: 3.0.x
Keywords: Cc:

Description

I confirmed this is an issue on my system as detailed in https://lists.osgeo.org/pipermail/postgis-users/2022-January/045186.html

If you uninstall an old version of PostGIS you were using and then install a new version (that doesn't use the same lib file), you get this error.

SELECT postgis_extensions_upgrade();
NOTICE:  Updating extension postgis from 3.1.4 to 3.2.0

ERROR:  could not access file "$libdir/postgis-3": No such file or directory
CONTEXT:  PL/pgSQL function _postgis_drop_function_if_needed(text,text) line 6 at FOR over SELECT rows
SQL statement "ALTER EXTENSION postgis UPDATE TO "3.2.0";"
PL/pgSQL function postgis_extensions_upgrade() line 79 at EXECUTE
SQL state: 58P01

I thought we had dealt with this issue before, to allow upgrade even in absense of old library.

Change History (15)

comment:1 by strk, 7 months ago

I wonder if this was introduced by our handling of deprecated functions, can you check if reverting to before that handling fixes the issue ? Doesn't Dronie test many upgrades already, btw ? Can this be a case not tested by Dronie ?

comment:2 by robe, 7 months ago

I think it would be missed by dronie because in her case she always has the prior version installed she is upgrading from.

We would need her to delete postgis she just installed before she upgrades to simulate the bevior.

comment:3 by strk, 7 months ago

To reproduce:

  1. Create a break_probin.sql file containing this statement: {{{ update pg_proc set probin = 'nonexistent' where probin like '%postgis%'; }}}
  2. run: regress/run_test.pl --extension --upgrade-path 2.5.6dev--:auto --before-upgrade-script break_probin.sql regress/core/regress.sql -v

Replace 2.5.6dev with whatever version you have available.

We could have Dronie do the above, but we need a fix first …

comment:4 by strk, 7 months ago

The problem seems to be that pg_get_function_arguments fail when passed the OID of some function having a non-existent probin (but not for all of them).

Example:

NOTICE:  Success calling pg_get_function_arguments for oid 22100136 (st_locatebetweenelevations(geometry,double precision,double precision)): geometry geometry, fromelevation double precision, toelevation double precision
NOTICE:  Success calling pg_get_function_arguments for oid 22100137 (st_interpolatepoint(geometry,geometry)): line geometry, point geometry
NOTICE:  Failed calling pg_get_function_arguments for oid 22100138 (st_hexagon(double precision,integer,integer,geometry))
NOTICE:  Success calling pg_get_function_arguments for oid 22100251 (geometry_spgist_inner_consistent_3d(internal,internal)): internal, internal

I wonder what's special about, for example, ST_Hexagon, which does not affect others.

These are actually the ONLY function for which pg_get_function_arguments raises an exception:

NOTICE:  Failed calling pg_get_function_arguments for oid 22099491 (st_angle(geometry,geometry,geometry,geometry))
NOTICE:  Failed calling pg_get_function_arguments for oid 22099549 (st_tileenvelope(integer,integer,integer,geometry,double precision))
NOTICE:  Failed calling pg_get_function_arguments for oid 22100138 (st_hexagon(double precision,integer,integer,geometry))
NOTICE:  Failed calling pg_get_function_arguments for oid 22100139 (st_square(double precision,integer,integer,geometry))

comment:5 by strk, 7 months ago

I guess the special thing about those functions is that they have a DEFAULT value expressed in a canonical text for type GEOMETRY:

CREATE OR REPLACE FUNCTION ST_Square(size float8, cell_i integer, cell_j integer, origin geometry DEFAULT 'POINT(0 0)')
CREATE OR REPLACE FUNCTION ST_Square(size float8, cell_i integer, cell_j integer, origin geometry DEFAULT 'POINT(0 0)')
CREATE OR REPLACE FUNCTION ST_Angle(pt1 geometry, pt2 geometry, pt3 geometry, pt4 geometry default 'POINT EMPTY'::geometry)
CREATE OR REPLACE FUNCTION ST_TileEnvelope(zoom integer, x integer, y integer, bounds geometry DEFAULT 'SRID=3857;LINESTRING(-20037508.342789244 -20037508.342789244, 20037508.342789244 20037508.342789244)'::geometry, margin float8 DEFAULT 0.0)

BTW, ST_TileEnvelope default looks weird to me (what's that?)

Anyway, it looks like pg_get_function_arguments (which is used by _postgis_drop_function_if_needed ) chokes when the default value of a geometry is given.

I guess this is because some function associated with the GEOMETRY type is invoked (typmod_in? input?)

comment:7 by Sandro Santilli <strk@…>, 7 months ago

In 2abdefd/git:

Test ugprades in absence of old library

See #5046

comment:8 by strk, 7 months ago

I pushed a test-upgrades-in-absence-of-old-library branch in official repository so we (and CI) can test it. In that branch, I'm breaking all C functions so they point at unexisting library.

CIs confirm the bug reported here: https://gitlab.com/postgis/postgis/-/jobs/1973927581#L15189

comment:9 by strk, 7 months ago

https://git.osgeo.org/gitea/postgis/postgis/pulls/72 is associated with the new branch, to report failures (idea: we could file a new PR on each mirror, against the *same* branch) to have all bots test it :)

comment:13 by strk, 7 months ago

I now pushed a branch in which we ALSO break the geometry_out function, so to have a confirmation of this bug. The bug is confirmed by:

GitHub actions are NOT catching the upgrade failure, due to $5054. Debbie and friends I'm not sure they kick for non standard branchs (Regina?)

What I think we could test in that branch is if rewriting the _postgis_drop_if_needed function can help (we'd basically need a way to find argument names without evaluating default values, although we MIGHT want to change default values in the future ?).

In general, it would probably be more robust if the defaults would always be NULL, although I do understand that means removing the STRICT and handling it at the library level…

Version 1, edited 7 months ago by strk (previous) (next) (diff)

comment:14 by Sandro Santilli <strk@…>, 7 months ago

In 95bedd94/git:

Rewrite _postgis_drop_if_needed to avoid expanding DEFAULT values

This prevents the function to fail when geometry canonical output
function is non-functional (maybe due to lack of library on system).

References #5046 in master branch

Includes regression testing, which now avoid keeping geometry_out
functional.

comment:15 by strk, 7 months ago

This should now be fixed in master branch. It's to be verified which other branches are affected by the issue (and I'd backport also regression tests)

comment:16 by strk, 7 months ago

Milestone: PostGIS 3.2.1PostGIS 3.0.5
Version: 3.2.x3.0.x

I went testing: we have this bug starting in PostGIS-3.0 branch (2.5 did not have the _postgis_drop_if_needed function) — I'm going to backport the fix

comment:17 by Sandro Santilli <strk@…>, 7 months ago

In ab601a0/git:

Rewrite _postgis_drop_if_needed to avoid expanding DEFAULT values

This prevents the function to fail when geometry canonical output
function is non-functional (maybe due to lack of library on system).

References #5046 in 3.2 branch (3.2.1dev)

Includes test ugprades in absence of old library
Includes NEWS item

comment:18 by Sandro Santilli <strk@…>, 7 months ago

In 2be8f572/git:

Rewrite _postgis_drop_if_needed to avoid expanding DEFAULT values

This prevents the function to fail when geometry canonical output
function is non-functional (maybe due to lack of library on system).

References #5046 in 3.1 branch (3.1.5dev)

Includes NEWS item
Include ugprades tests in absence of old library

comment:19 by Sandro Santilli <strk@…>, 7 months ago

Resolution: fixed
Status: newclosed

In 4cb6c89/git:

Rewrite _postgis_drop_if_needed to avoid expanding DEFAULT values

This prevents the function to fail when geometry canonical output
function is non-functional (maybe due to lack of library on system).

Closes #5046 in 3.0 branch (3.0.5dev)

Includes NEWS item
Include ugprades tests in absence of old library

Note: See TracTickets for help on using tickets.