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

[robe]

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

[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();

[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.

[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

[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

[Raúl Marín <git@…>]

In 94e5b05/git:

Fix pg_upgrade issue with st_linecrossingdirection

Closes #4590

[strk]

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.

[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.

[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 ?

[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.

[strk]

Ok thanks for the review :)