Opened 4 months ago

Closed 4 months ago

#4590 closed defect (fixed)

Upgrading PostGIS 3.0 PG 11 to PostGIS 3.0 PG 12 errors after pg_upgrade - st_linecrossingdirection

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

Description

Might be something particular about this database, I'm going to try another.

Here is the situation

1) Had a PostgreSQL 11.5 database which I had upgraded to PostGIS 3.0.0 from PostGIS 2.5.2 (or 2.5.1, can't remember)

2) Ran pg_upgrade on the server to bring it to PostgreSQL 12, PostGIS 3.0.0

pg_upgrade went fine. After upgrade

SELECT postgis_full_version();

Gives:

POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="110" (procs need upgrade for use with PostgreSQL "120") GEOS="3.8.0-CAPI-1.13.1 " PROJ="Rel. 5.2.0, September 15th, 2018" LIBXML="2.9.9" LIBJSON="0.12" LIBPROTOBUF="1.2.1" WAGYU="0.4.3 (Internal)"

So went to remedy the 11/12 situation by:

SELECT postgis_extensions_upgrade();

and got:

ERROR:  cannot change name of input parameter "line1"
HINT:  Use DROP FUNCTION st_linecrossingdirection(geometry,geometry) first.
CONTEXT:  SQL statement "ALTER EXTENSION postgis UPDATE TO "3.0.0next";"
PL/pgSQL function postgis_extensions_upgrade() line 69 at EXECUTE
SQL state: 42P13

I don't recall us renaming the args for this

Change History (10)

comment:1 Changed 4 months ago by robe

To work around the issue I did this:

ALTER EXTENSION postgis DROP FUNCTION st_linecrossingdirection(geometry,geometry);
DROP FUNCTION st_linecrossingdirection(geometry,geometry);
SELECT postgis_extensions_upgrade();

comment:2 Changed 4 months ago by Algunenano

There is an issue with the ifdef PG11. For PG11 and before it currently has line1 and line2 as parameters, while for PG12 it has geom1 and geom2.

PG11:

CREATE OR REPLACE FUNCTION ST_LineCrossingDirection(line1 geometry, line2 geometry)
	RETURNS integer AS
	$$ SELECT CASE WHEN NOT $1 OPERATOR(@extschema@.&&) $2 THEN 0 ELSE @extschema@._ST_LineCrossingDirection($1,$2) END $$
	LANGUAGE 'sql' IMMUTABLE PARALLEL SAFE;

PG12:

CREATE OR REPLACE FUNCTION ST_LineCrossingDirection(geom1 geometry, geom2 geometry)
	RETURNS integer
	AS 'MODULE_PATHNAME', 'ST_LineCrossingDirection'
	SUPPORT postgis_index_supportfn
	LANGUAGE 'c' IMMUTABLE STRICT PARALLEL SAFE
	_COST_HIGH;

The comment in postgis_before_upgrade.sql says:

-- FUNCTION ST_LineCrossingDirection changed argument names in 3.0
-- Was (geom1 geometry, geom2 geometry) and now (line1 geometry, line2 geometry)
SELECT _postgis_drop_function_if_needed
	(
	'ST_LineCrossingDirection',
	'geom1 geometry, geom2 geometry'
	);

-- FUNCTION _st_linecrossingdirection changed argument names in 3.0
-- Was (geom1 geometry, geom2 geometry) and now (line1 geometry, line2 geometry)
SELECT _postgis_drop_function_if_needed
	(
	'_ST_LineCrossingDirection',
	'geom1 geometry, geom2 geometry'
	);

So I guess it's an oversight and the PG12 version should also have line1 and line2 as arguments.

comment:3 Changed 4 months ago by Algunenano

Let's fix this before 3.0.1: https://github.com/postgis/postgis/pull/519 The exceptions in postgis_before_upgrade.sql should be enough the handle the change

comment:4 Changed 4 months ago by Raúl Marín <git@…>

In 1ceef38/git:

Fix pg_upgrade issue with st_linecrossingdirection

References #4590
Closes https://github.com/CartoDB/Windshaft-cartodb/pull/1143

comment:5 Changed 4 months ago by Raúl Marín <git@…>

Resolution: fixed
Status: newclosed

In 94e5b05/git:

Fix pg_upgrade issue with st_linecrossingdirection

Closes #4590

comment:6 Changed 4 months ago by strk

Resolution: fixed
Status: closedreopened

I'm confused by the changeset, I see:

-- Availability: 1.4.0

-CREATE OR REPLACE FUNCTION ST_LineCrossingDirection(geom1 geometry, geom2 geometry) +CREATE OR REPLACE FUNCTION ST_LineCrossingDirection(line1 geometry, line2 geometry)

RETURNS integer

Which means a function signature is changed. For a function which is advertised as being available since PostGIS 1.4.0. What's the deal, could you summarize what happened ? Did we ship a broken version of PostGIS which changed signature without adding a -- Changed comment ?

Changing the signature back means whoever installed the broken PostGIS would fail upgrading again.

comment:7 Changed 4 months ago by Algunenano

What's the deal, could you summarize what happened ?

It's explained above, the signature changed between 2.5 and 3.0 but only for PG11 and before.

Did we ship a broken version of PostGIS which changed signature without adding a -- Changed comment ?

Yes and no. We shipped a PostGIS release that changed a signature without adding a -- Changed comment on top. That doesn't mean it's broken.

Changing the signature back means whoever installed the broken PostGIS would fail upgrading again.

I didn't change the signature back, I changed it forward to match the one used for PG11. Neither the signature is broken nor it will fail on upgrade again as far as I can see.

comment:8 Changed 4 months ago by strk

We shipped a PostGIS release that changed a signature without adding a -- Changed comment on top. That doesn't mean it's broken.

It means upgrades would fail, doesn't it ? And in order to complete an upgrade one would have to DROP the previous function ? If that's the case, whoever fixed the upgrade from 2.5 would end up being unable to upgrade again ?

comment:9 Changed 4 months ago by Algunenano

It means upgrades would fail, doesn't it ?

Obviously not, since people have already migrated from 2.5 to 3.0. It was handled using ./postgis/postgis_before_upgrade.sql and that's still in place, so upgrades that require changing the signature from geom1 to line1 don't fail.

Again, the issue here is that 3.0+PG11 -> 3.0+PG12 was changing the signature from the new one to the old one (due to this bug) and we don't have a way to handle this explicit change, but we do handle the change from the old signature to the new one.

comment:10 Changed 4 months ago by strk

Resolution: fixed
Status: reopenedclosed

Ok thanks for the review :)

Note: See TracTickets for help on using tickets.