Unqualified function calls break postgres_fdw compatibiliy

[fphilipe]

I'm using postgres_fdw to access a database that has postgis installed. The problem is that the session that postgres_fdw opens only has pg_catalog in its search_path.

Let's call the DB being accessed by the foreign data wrapper (FDW) db1 and the DB hosting the FDW db2.

I have a view in db1 that calculates a distance between to geography points. In db2 I have a foreign table that reads from that view. Unfortunately the call to st_distance inside the FDW session fails because st_distance calls a private function _st_distance without explicitly specifying the schema.

Attached is a minimal script that reproduces this bug. Altering the function st_distance such that the schema postgis is installed in is in its search path fixes the problem. Unfortunately this is not always a feasible option.

Version details:

PostgreSQL: PostgreSQL 9.4.0 on x86_64-apple-darwin14.0.0, compiled by Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn), 64-bit PostGIS: POSTGIS="2.1.5 r13152" GEOS="3.4.2-CAPI-1.8.2 r3921" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.11.1, released 2014/09/24" LIBXML="2.9.2" LIBJSON="UNKNOWN" RASTER

[fphilipe]

Minimal example reproducing the described bug

[robe]

This is the same case as #2485, and some others we have in the system (which I can't find at the moment) so not specific to postgres_fdw. The issue is with PostGIS functions that call other PostGIS functions or tables.

I was thinking about this a lot lately and thinking the only none-breaking change we could make was to as part of the function, add the common schemas people install these in to the search path of the functions in question.

It's only functions that reference other functions (or tables) that are affected by this issue. Things like ST_Distance, a bunch of the raster contraint functions, ST_Transform, ST_Buffer, ST_DWithin.

I have to bring it up for vote and also do some more tests with this idea, but as far as I can tell, PostgreSQL is okay with specifying non-existent schemas in the search path.

So if we go with the standard schemas people install PostGIS in — public, postgis, contrib we should be okay. I haven't run across any other schemas people use to install postgis in, but doing such a thing would be a breaking change for these folks.

[fphilipe]

not specific to postgres_fdw

That's true. Just wanted to show a context in which there is no workaround since there's no way to change the search_path.

Another minimal way to reproduce this:

CREATE EXTENSION postgis;
SET search_path TO pg_catalog;
select public.st_distance('POINT(0 0)'::public.geography, 'POINT(1 1)'::public.geography);

[fphilipe]

Replying to robe:

So if we go with the standard schemas people install PostGIS in — public, postgis, contrib we should be okay. I haven't run across any other schemas people use to install postgis in, but doing such a thing would be a breaking change for these folks.

FYI, I usually create it in a schema called extensions.

[robe]

oh no yet another. This might require a user survey.

[rdunklau]

If I understand the proposed fix correctly, it would involve hard-coding a set of "often used schemas".

IMHO, this is really error-prone: either postgis is relocatable, and any schema should be good, or it is not, and a fixed schema should be used.

Since postgis pretends to be relocatable, it should use the user-specified schema correctly, by using the @extschema@ placeholder.

[robe]

rdunklau,

Yap that's the path we are going down.

See this thread - ​http://lists.osgeo.org/pipermail/postgis-devel/2015-March/024806.html

The only issue I suspect we'll run into is if the person then relocates the extension again, I think the old one would be stuck because I suspect the extension machinery would then hard-code this new schema at extension install time and ALTER EXTENSIO SET SCHEMA would then use the old path making the functions unusable. I have to test that theory, but I suspect that will be the downside of using only @extschema@

[rdunklau]

Thanks for clearing that up !

Regards

[robe]

Sadly this requires my plan to put in search_path and I missed curtain call for 2.2 on that so I think this will have to wait till 2.3

[robe]

I think I have a plan. This is a mockup of it,

​https://trac.osgeo.org/postgis/browser/trunk/postgis/postgis_functions_search_path.sql

but once I test this I'll be braver and just have it generate a script to do this for all functions (messing with strks' perl scripts to generate this additional script (one for postgis proper and one for raster).

For 2.3, we can fold this in the create extension, update extension process. For older, I'm just going to have it as a stand-alone script with an FAQ that people should run the script after every install and upgrade to be able to easily restore their backups and also make there fdw and materialized views work.

Sadly there is a downside. Downside being I can't have the search path change in the ALTER EXTENSION .. SCHEMA change. Unless I introduce event triggers (which someone in code-sprint suggested) which pramsey at codesprint gave a sour face too.

[strk]

Please let's keep things easy. I think schema should be set at build time and never change at run time. Compile-time defined schema should then be used for both extension and standalone scripts.

Shall anyone want to use a different schema, we could provide a script to do it.

For extensions we should research what it means to upgrade from a version to another when the schema changes (if possible at all).

If changing schema on upgrade is easy and feasible, we could take the chance to move to a "postgis" schema by default.

In any case, it would be very helpful to setup automated upgrade tests on the bots (both soft and hard, see #2676 and #3444 for a start but it's not complete)

[robe]

strk,

Even if we had at build time, we still need to set search_path of all functions. Only gain is we don't need to worry about ALTER EXTENSION SET SCHEMA. That said I'd rather keep at runtime for now, because the only sane solution is to force everyone to install in same schema - postgis, which is liable to break a lot of applications and we need a solution now because people can't restore their databases.

[robe]

Refer to #3490.

I'm leaving this still open since my script will only partially fix this.

To 100% fix, we'll need to schema qualify the functions that rely on SQL inlining. Which getting back to your point strk, we'd have to enforce at lease build-time schema.

[robe]

will close this out after done with #3496 which will solve this issue for all

[robe]

In 15041:

schema qualify function and operator calls in geometry and geography functions

Closes #3496
Closes #3494
Closes #3486
Closes #3076