Context Navigation

← Previous Ticket
Next Ticket →

#5569 closed defect (fixed)

postgis_restore doesn't strip 2.1 function signatures

Reported by:	kesar	Owned by:	strk
Priority:	low	Milestone:	PostGIS 3.4.1
Component:	build	Version:	3.4.x
Keywords:	postgis_restore	Cc:	pramsey

Description

Hi,

we are migrating an old database to a new server and doing the HARD UPGRADE (as instructed in http://postgis.net/docs/manual-3.4/postgis_administration.html#hard_upgrade) from Postgis 2.1.8 to 3.4.

Seems that some deprecated/old functions are not stripped by postgis_restore:

CREATE FUNCTION pgis_abs_in(cstring) RETURNS pgis_abs
    LANGUAGE c IMMUTABLE STRICT
    AS '$libdir/postgis-2.1', 'pgis_abs_in';

CREATE FUNCTION pgis_abs_out(pgis_abs) RETURNS cstring
    LANGUAGE c IMMUTABLE STRICT
    AS '$libdir/postgis-2.1', 'pgis_abs_out';

CREATE FUNCTION pgis_geometry_accum_finalfn(pgis_abs) RETURNS geometry[]
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_accum_finalfn';


CREATE FUNCTION pgis_geometry_accum_transfn(pgis_abs, geometry) RETURNS pgis_abs
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_accum_transfn';


CREATE FUNCTION pgis_geometry_collect_finalfn(pgis_abs) RETURNS geometry
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_collect_finalfn';

CREATE FUNCTION pgis_geometry_makeline_finalfn(pgis_abs) RETURNS geometry
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_makeline_finalfn';

CREATE FUNCTION pgis_geometry_polygonize_finalfn(pgis_abs) RETURNS geometry
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_polygonize_finalfn';

CREATE FUNCTION pgis_geometry_union_finalfn(pgis_abs) RETURNS geometry
    LANGUAGE c
    AS '$libdir/postgis-2.1', 'pgis_geometry_union_finalfn';

CREATE FUNCTION st_askml(geog geography, maxdecimaldigits integer DEFAULT 15) RETURNS text
    LANGUAGE sql IMMUTABLE STRICT
    AS $_$SELECT _ST_AsKML(2, $1, $2, null)$_$;


CREATE FUNCTION st_askml(geom geometry, maxdecimaldigits integer DEFAULT 15) RETURNS text
    LANGUAGE sql IMMUTABLE STRICT
    AS $_$ SELECT _ST_AsKML(2, ST_Transform($1,4326), $2, null); $_$;


CREATE FUNCTION st_dwithin(geography, geography, double precision) RETURNS boolean
    LANGUAGE sql IMMUTABLE
    AS $_$SELECT $1 && _ST_Expand($2,$3) AND $2 && _ST_Expand($1,$3) AND _ST_DWithin($1, $2, $3, true)$_$;

It also tries to add comments to non-existent functions and constraints to already existing Postgis 3.4 tables.

Here are the error messages:

ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_abs_in(cstring) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_abs_out(pgis_abs) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_accum_finalfn(pgis_abs) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_accum_transfn(pgis_abs, geometry) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_collect_finalfn(pgis_abs) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_makeline_finalfn(pgis_abs) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_polygonize_finalfn(pgis_abs) does not exist
ERROR:  could not access file "$libdir/postgis-2.1": No such file or directory
ERROR:  function public.pgis_geometry_union_finalfn(pgis_abs) does not exist
ERROR:  function st_asgeojson(integer, geography, integer, integer) does not exist
ERROR:  function st_asgeojson(integer, geometry, integer, integer) does not exist
ERROR:  function st_askml(integer, geography, integer, text) does not exist
ERROR:  function st_count(text, text, boolean) does not exist
ERROR:  function st_count(text, text, integer, boolean) does not exist
ERROR:  function st_force3d(geometry) does not exist
ERROR:  function st_force3dm(geometry) does not exist
ERROR:  function st_force3dz(geometry) does not exist
ERROR:  function st_force4d(geometry) does not exist
ERROR:  function st_histogram(text, text, integer, integer, boolean) does not exist
ERROR:  function st_histogram(text, text, integer, boolean, integer, boolean) does not exist
ERROR:  function st_histogram(text, text, integer, integer, double precision[], boolean) does not exist
ERROR:  function st_histogram(text, text, integer, boolean, integer, double precision[], boolean) does not exist
ERROR:  function st_quantile(text, text, integer, double precision[]) does not exist
ERROR:  function st_quantile(text, text, integer, boolean, double precision[]) does not exist
ERROR:  function st_summarystats(text, text, boolean) does not exist
ERROR:  function st_summarystats(text, text, integer, boolean) does not exist
ERROR:  function validatetopology(character varying) does not exist
ERROR:  multiple primary keys for table "layer" are not allowed
ERROR:  relation "layer_schema_name_table_name_feature_column_key" already exists
ERROR:  relation "topology_name_key" already exists
ERROR:  multiple primary keys for table "topology" are not allowed
ERROR:  rule "geometry_columns_delete" for relation "geometry_columns" already exists
ERROR:  rule "geometry_columns_insert" for relation "geometry_columns" already exists
ERROR:  rule "geometry_columns_update" for relation "geometry_columns" already exists
ERROR:  trigger "layer_integrity_checks" for relation "layer" already exists
ERROR:  constraint "layer_topology_id_fkey" for relation "layer" already exists
ERROR:  multiple primary keys for table "spatial_ref_sys" are not allowed

Attachments (1)

postgis21_manifest.txt (110.5 KB ) - added by kesar 13 months ago.: output from pg_restore -l, non-postgis information redacted

Download all attachments as: .zip

Change History (27)

comment:1 by strk, 13 months ago

Can you share the manifest of your dump ? You obtain it with pg_restore -l <yourdumpfile>

comment:2 by strk, 13 months ago

Also please report the full target version from which postgis_restore was built (unfortunately we never added a --version switch to it so you'll have to figure out in some other way)

comment:3 by strk, 13 months ago

Cc:	prasey added

The pgis_abs_in signature was deprecated in 2.5.0 according to postgis/postgis_legacy.c and there's a mention in it in doc/rfc/postgis_rfc_03_sheet.txt that I don't understand (rfc03 was commited by Paul in 2009: ideas Paul?)

What's clearly missing is a DROP of that function in any of the upgrade scripts, which means whoever is upgrading from a version < 2.5.0 might have that function left in the database. The regress/core/regress.sql test should catch this case reporting unexpected probin but none of our bots are testing upgrades from 2.4.x: oldest upgrade is tested by woodie from 2.5.9: https://woodie.osgeo.org/repos/30/pipeline/1305/14

So, Paul or Regina: any idea why we'd ever want to keep those old functions rather than dropping them on upgrade (adding them to postgis_after_upgrade or something ?)

The only reason for that I can think of is that our upgrade script is not able to change the "geometry" TYPE definition and thus will STILL use the old functions, but it would not seem a good reason to keep old function around to me (rather we should fix the upgrade)

by kesar, 13 months ago

Attachment:	postgis21_manifest.txt added

output from pg_restore -l, non-postgis information redacted

comment:4 by kesar, 13 months ago

Hi & thank you!

OK, I added a sanitized version of the manifest output as an attachment.

The version of postgis_restore we are using is installed from Postgresql.org RHEL8 pgdg13-repository's package postgis34_13

$ dnf info 'postgis34_13' | grep Source
Source       : postgis34_13-3.4.0-1PGDG.rhel8.src.rpm

comment:5 by strk, 13 months ago

Looking at the generated DATA section in postgis_restore.pl, and focusing to a single signature, we have:

In manifest:

6714; 0 0 COMMENT public FUNCTION st_asgeojson(gj_version integer, geog geography, maxdecimaldigits integer, options integer) postgres 1019; 1255 304728 FUNCTION public st_asgeojson(integer, geography, integer, integer) postgres

In skip list:

COMMENT FUNCTION st_asgeojson(version integer, geog geography, maxdecimaldigits integer, options integer) FUNCTION st_asgeojson(integer, geography, integer, integer)

Sounds like our upgrade procedure is getting some parameter names wrong

kesar: are you able by any chance to provide also a schema-only dump for us to test restoring ?

Version 0, edited 13 months ago by strk (next)

comment:6 by strk, 13 months ago

The source of our st_asgeojson(version integer... SKIP line is postgis/legacy.sql.in which is where that signature is found. There's no mention anywhere about a gj_version integer parameter ever being found in postgis, for ST_AsGeoJSON

comment:7 by strk, 13 months ago

The ST_AsGeoJson(gj_version int4, geog geography, maxdecimaldigits int4 DEFAULT 15, options int4 DEFAULT 0) signature was present in 2.5 branch, dropped in 3.0

comment:8 by strk, 13 months ago

The actual source is postgis/uninstall_legacy.sql which drops the "version integer" but not the "gj_version integer". I guess we want to enhance utils/create_skip_signatures.pl to also parse _postgis_drop_function_by_identity as that's where the knowledge about obsoleted functions is now encoded

comment:9 by strk, 13 months ago

I would add that this ST_AsGeoJSON case should emerge in upgrades dump-reload based upgrades from 2.5 but evidently none of our bots are testing that ! Commit [ecea1b90e42943d78affaf8ddb96dbacc198f9a9/git] added support for those tests in run_test.pl so we want to enable them in CI.

I filed https://trac.osgeo.org/postgis/ticket/5573 for this

comment:10 by strk, 13 months ago

The ST_AsGeoJSON issue is now confirmed by CI: https://woodie.osgeo.org/repos/30/pipeline/1324/7#L9821

comment:11 by strk, 13 months ago

With recent commits we got CI successfully test of hard-upgrade procedure with a dump of a database created via script from 2.5.9 -- the code is in the current stable-3.4 branch. Your dump is pretty old, coming from 2.1. Our CI infrastructure is not ready to test 2.1 and we don't even have anymore the information about which PostgreSQL version worked with 2.1 ( 2.5 is the oldest we have info for: https://trac.osgeo.org/postgis/wiki/UsersWikiPostgreSQLPostGIS ).

Getting our hands on that structure-only dump could be useful to run some manual tests

comment:12 by strk, 13 months ago

Cc:	pramsey added; prasey removed
Summary:	postgis_restore doesn't strip some old functions → postgis_restore doesn't strip 2.1 function signatures

comment:13 by strk, 13 months ago

I found a way to test without having the dump:

touch x;
postgis_restore -v -L postgis21_manifest.txt x 2>&1 | grep KEEP

we will not want any postgis object in the output.

With the current code the left over are:

KEEP: SHELLTYPE pgis_abs
KEEP: FUNCTION pgis_abs_in(cstring)
KEEP: FUNCTION pgis_abs_out(pgis_abs)
KEEP: FUNCTION pgis_geometry_accum_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_accum_transfn(pgis_abs, geometry)
KEEP: FUNCTION pgis_geometry_collect_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_makeline_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_polygonize_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_union_finalfn(pgis_abs)

comment:14 by strk, 13 months ago

In addition to those functions reported below, we also have a lot of unexpected COMMENT retained which shouldnt be:

KEEP: COMMENT FUNCTION st_asgeojson(gj_version integer, geog geography, maxdecimaldigits integer, options integer)
KEEP: COMMENT FUNCTION st_asgeojson(gj_version integer, geom geometry, maxdecimaldigits integer, options integer)
KEEP: COMMENT FUNCTION st_askml(geog geography, maxdecimaldigits integer)
KEEP: COMMENT FUNCTION st_askml(geom geometry, maxdecimaldigits integer)
KEEP: COMMENT FUNCTION st_askml(version integer, geog geography, maxdecimaldigits integer, nprefix text)
KEEP: COMMENT FUNCTION st_aslatlontext(geometry, text)
KEEP: COMMENT FUNCTION st_buffer(geometry, double precision, integer)
KEEP: COMMENT FUNCTION st_buffer(geometry, double precision, text)
KEEP: COMMENT FUNCTION st_count(rastertable text, rastercolumn text, exclude_nodata_value boolean)
KEEP: COMMENT FUNCTION st_count(rastertable text, rastercolumn text, nband integer, exclude_nodata_value boolean)
KEEP: COMMENT FUNCTION st_coveredby(geography, geography)
KEEP: COMMENT FUNCTION st_covers(geography, geography)
KEEP: COMMENT FUNCTION st_distance(geography, geography, boolean)
KEEP: COMMENT FUNCTION st_dwithin(geography, geography, double precision, boolean)
KEEP: COMMENT FUNCTION st_histogram(rastertable text, rastercolumn text, nband integer, bins integer, "right" boolean, OUT min double precision, OUT max double precision, OUT count bigint, OUT percent double precision)
KEEP: COMMENT FUNCTION st_histogram(rastertable text, rastercolumn text, nband integer, exclude_nodata_value boolean, bins integer, "right" boolean, OUT min double precision, OUT max double precision, OUT count bigint, OUT percent double precision)
KEEP: COMMENT FUNCTION st_histogram(rastertable text, rastercolumn text, nband integer, bins integer, width double precision[], "right" boolean, OUT min double precision, OUT max double precision, OUT count bigint, OUT percent double precision)
KEEP: COMMENT FUNCTION st_histogram(rastertable text, rastercolumn text, nband integer, exclude_nodata_value boolean, bins integer, width double precision[], "right" boolean, OUT min double precision, OUT max double precision, OUT count bigint, OUT percent double precision)
KEEP: COMMENT FUNCTION st_intersects(geography, geography)
KEEP: COMMENT FUNCTION st_isvaliddetail(geometry, integer)
KEEP: COMMENT FUNCTION st_linecrossingdirection(geom1 geometry, geom2 geometry)
KEEP: COMMENT FUNCTION st_orderingequals(geometrya geometry, geometryb geometry)
KEEP: COMMENT FUNCTION st_quantile(rastertable text, rastercolumn text, nband integer, quantiles double precision[], OUT quantile double precision, OUT value double precision)
KEEP: COMMENT FUNCTION st_quantile(rastertable text, rastercolumn text, nband integer, exclude_nodata_value boolean, quantiles double precision[], OUT quantile double precision, OUT value double precision)
KEEP: COMMENT FUNCTION st_setsrid(geometry, integer)
KEEP: COMMENT FUNCTION st_srid(geometry)
KEEP: COMMENT FUNCTION st_summarystats(rastertable text, rastercolumn text, exclude_nodata_value boolean, OUT count bigint, OUT sum double precision, OUT mean double precision, OUT stddev double precision, OUT min double precision, OUT max double precision)
KEEP: COMMENT FUNCTION st_summarystats(rastertable text, rastercolumn text, nband integer, exclude_nodata_value boolean, OUT count bigint, OUT sum double precision, OUT mean double precision, OUT stddev double precision, OUT min double precision, OUT max double precision)
KEEP: COMMENT FUNCTION validatetopology(toponame character varying)

comment:15 by strk, 13 months ago

Ticket #3078 deals with the enhancement of not keeping those function comments.

comment:16 by strk, 13 months ago

With the changes in https://git.osgeo.org/gitea/postgis/postgis/pulls/160 the only left over signatures are:

KEEP: SHELLTYPE pgis_abs
KEEP: FUNCTION pgis_abs_in(cstring)
KEEP: FUNCTION pgis_abs_out(pgis_abs)
KEEP: FUNCTION pgis_geometry_accum_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_accum_transfn(pgis_abs, geometry)
KEEP: FUNCTION pgis_geometry_collect_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_makeline_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_polygonize_finalfn(pgis_abs)
KEEP: FUNCTION pgis_geometry_union_finalfn(pgis_abs)

comment:17 by strk, 13 months ago

pgis_abs_in and pgis_abs_out were functions used for the pgis_abs datatype up to PostGIS-2.4, deprecated in PostGIS-2.5 -- drop of which was handled in #4035 but then somehow moved to postgis_legacy.c as part of #4193

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

In a6d5ce0/git:

Simplify handling of comment signatures

Always skip comments on skipped objects.
Stop including now unneded comment signatures
in the installed postgis_restore script, reducing
its size by 65% (163k)

In master branch (3.5.0dev):

References #3078 #5569

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

In bc51b1a6/git:

Simplify handling of comment signatures

Always skip comments on skipped objects.
Stop including now unneded comment signatures
in the installed postgis_restore script, reducing
its size by 65% (163k)

In 3.4 branch (3.4.1dev):