ST_Transform references spatial_ref_sys without schema

[javitonino]

When trying to pg_restore a dump with some index that uses ST_Transform (CREATE INDEX ON schema.tab (st_transform(geom,3857)); ) you end up getting errors that spatial_ref_sys does not exists.

Steps to reproduce:

psql -U postgres
create database db;
\c db
create extension postgis;
create schema sc;
create table sc.tab(geom geometry);
create index on sc.tab (st_transform(geom,3857));
insert into sc.tab values (st_setsrid(st_point(0,0), 4326));
\q
pg_dump -U postgres db -Fc > dump
dropdb -U postgres db
createdb -U postgres db
pg_restore -U postgres -d db dump

pg_restore: [archiver (db)] Error from TOC entry 3435; 1259 15521376 INDEX tab_st_transform_idx1 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "spatial_ref_sys" does not exist
LINE 1: SELECT proj4text FROM spatial_ref_sys WHERE srid = 4326 LIMI...

I think the issue is that the SQL query is run from C code, where it has not been qualified with @extschema@ like the rest of the code since 2.3, causing the query to fail when run from pg_restore (which sets the search_path to the schema of the objects it's restoring).

[pramsey]

Well, we cannot use the @extschema@ trick here, as it's inside the C code. I wonder if we can use the fact that we're being executed within a function to find out what schema that function is installed in. That could end up being a lot of overhead on the transform code to look that up on every call, but it's the only way I can see to keep the extension relocatable.

[pramsey]

OK, I can see two fixes right now, both equally ugly in their own way.

Fix 1:

• Change st_transform and postgis_transform_geometry into SQL wrappers move the actual C bindings into _st_transform and _postgis_transform_geometry.
• In the wrappers, call the C bindings but include @extschema@ as an argument, so that we inject the schema into the C call.
• Potentially cleaner but still ugly variant: change the signature of st_transform and postgis_transform_geometry to have an extra 'schema' parameter that has a default value of @extschema@, then we can avoid the extra level of indirection and get the install schema injected still. Depends on defaults working and involves changing signature of existing heavily used function.

Fix 2:

• Go into the C guts of st_transform and postgis_transform_geometry.
• The FunctionCallInfo includes the Oid of the function being called.
• In the PgSQL syscache, look up the namespace that is associated with that function Oid.

Both fix 1 and fix 2 will require changing a lot of proj lookup code to take not just the SRID number being looked up, but also the schema to look for spatial_ref_sys in. After about 6 levels of calling in, the schema name can be inserted into the "select proj4text from %s.spatial_ref_sys where srid = %d" SPI query.

[pramsey]

Fix 3:

• From my colleague @javitonino, we can just add SET SEARCH_PATH = @extschema@, pg_catalog in the function definition itself. As the manual for ​https://www.postgresql.org/docs/9.5/static/sql-createfunction.html create function notes, we can add SET clauses there.

Unless someone has a screaming reason why this is not right, I think st_transform and postgis_transform_geometry can be upgraded with this small fix, and that should probably be backported as well, so other fresh installs get the right schema qualifications right up fron..

[robe]

Fix 3 was the one I had planned to do a while ago and actually had it in there before I backed it out.

The only reason not to put it in is the setting search_path in the function screws the planner and in ways I couldn't easily predict. So I didn't want to risk it for a particular problem that didn't seem to be affecting that many people.

I'm not totally against the idea, but think we'll need to do some benchmarking to see how badly if at all it screws up performance.

[robe]

Replying to pramsey:

• Potentially cleaner but still ugly variant: change the signature of st_transform and postgis_transform_geometry to have an extra 'schema' parameter that has a default value of @extschema@, then we can avoid the extra level of indirection and get the install schema injected still. Depends on defaults working and involves changing signature of existing heavily used function.

Wondering is it any easier to look this value up in a GUC variable than going thru FunctionCallInfo.

My thought is on install or upgrade of PostGIS, we can have our code set a guc variable to @extschema@.

Then if that guc variable is set, our SPI queries will use the GUC variable

[robe]

Just to add to this, don't forget raster has an ST_Transform too, so that would need to be patched as well.

[pramsey]

Just to add to this, don't forget raster has an ST_Transform too, so that would need to be patched as well.

Could just stick the install schema into a global in the backend. First call through, if it's null, set it. No user visible GUC required. Then the SPI lookup could check and see if the global is non-null and use it to qualify the query if it is. Ugly but avoids re-writing all the function signatures between through the proj cache.

[pramsey]

Confirmed that using search_path parameter on the function has a deleterious effect on performance.

create table pts as 
  select st_setsrid(st_makepoint(random()*360 - 180,random() * 180 - 90),4326) as geom, 
  generate_series(1,100000) as id;

select sum(st_x(st_transform(geom, 3857))) from pts;
-- Time: 157.676 ms

alter function st_transform(geometry, integer) set search_path=public,pg_catalog;

select sum(st_x(st_transform(geom, 3857))) from pts;
-- Time: 587.814 ms

Very unfortunate.

[pramsey]

In 16262:

When looking up SRID numbers, assume that spatial_ref_sys will
share a schema will the calling function (st_transform, usually)
and look for spatial_ref_sys there. (References #3975)

[pramsey]

In 16271:

2.4 fix, st_transform does not qualify query to spatial_ref_sys (references #3975)

[pramsey]

In 16272:

2.3 fix, st_transform does not qualify query to spatial_ref_sys (references #3975)

[robe]

pramsey if you've already committed this is there a reason this ticket is still open?