On Fri, Mar 30, 2018 at 04:52:29PM -0400, Bruce Momjian wrote:> On Wed, Mar 14, 2018 at 11:12:26PM +0000, PG Bug reporting form wrote:> > The following bug has been logged on the website:> > > > Bug reference: 15112> > Logged by: Keiko Oda> > Email address: keiko713(at)gmail(dot)com> > PostgreSQL version: 10.3> > Operating system: Ubuntu> > Description: > > > > This is similar to the bug report from 2016> > (https://www.postgresql.org/message-id/20161015001424.1413.93990@wrigleys.postgresql.org).> > where earthdistance extension is assuming that it's executed with public> > schema in search_path.> > > > With 10.3 release, Postgres has tighten the search_path during pg_upgrade> > only to pg_catalog. This is problematic as it's now impossible to run> > pg_upgrade with earthdistance extension (it fails with create index with> > ll_to_earth function).> > Prior to this, we were at least able to workaround by adding public schema> > in search_path with pg_upgrade.> > > > As it's recommended in the release note, earthdistance should adjust these> > functions to not assume anything about what search path they are invoked> > under.> > Uh, I can reproduce this failure. :-(> > I tested it by installing earchdistance (and cube) and an index using an> earchdistance function in the old cluster and ran pg_upgrade. I used> the instructions in the referenced email:> > CREATE TABLE zip_code_geo_poly_data (> id serial NOT NULL PRIMARY KEY,> zip_code TEXT,> latitude NUMERIC,> longitude NUMERIC> );> > CREATE INDEX zip_code_geo_poly_data_ll_to_earth > ON zip_code_geo_poly_data > USING gist(ll_to_earth(latitude, longitude));> > The failure is actually in pg_dump/restore, but pg_upgrade relies on> that, so it fails too. The failure output is:> > LINE 1: ...ians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth> ^> QUERY: SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth> CONTEXT: SQL function "ll_to_earth" during inlining> Command was:> -- For binary upgrade, must preserve pg_class oids> SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16498'::pg_catalog.oid);> > CREATE INDEX "zip_code_geo_poly_data_ll_to_earth" ON "public"."zip_code_geo_poly_data" USING "gist" ("public"."ll_to_earth"(("latitude")::double precision, ("longitude")::double precision));> > The function definition of ll_to_earth() is (as defined in> earthdistance--1.1.sql):> > CREATE FUNCTION ll_to_earth(float8, float8)> RETURNS earth> LANGUAGE SQL> IMMUTABLE STRICT> PARALLEL SAFE> AS 'SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth';> > As you can see, the SQL function is doing a cast to ::earth, but in this> test case the earth cast is stored in the public schema, and the restore> doesn't reference it.> > I am not sure we can fix this without requiring people to drop and> recreate such indexes. However, I am even at a loss in how to fix the> CREATE FUNCTION to reference a cast in the same schema as the function,> in this case 'public'. We can rewrite the cast to not use :: and use a> function call with schema qualification. e.g. public.earth(), but how do> we know what schema that is in, i.e. what if the extension is loaded> into a schema other than public? > > FYI, earthdistance is certainly not the only case of this problem. > > This is also part of a larger problem with our new schema qualification> approach. While we are qualifying where the object is to be created, we> are not always qualifying the object's contents, i.e. in this case, the> SQL function body.

I have not received a reply to this so I am CCing the hackers list formore input. (If this is not the proper procedure, please let me know.)

The only additional thoughts I have are that when we removed public frompg_dump's search_path and decided to instead fully qualify objectcreation names, I was concerned about function creation failing becauseof 'public' schema object references in the function body, but someonepointed out that we don't check function bodies during restore becauseof:

SET check_function_bodies = false;

However, what I did not consider was creation of indexes based onfunction bodies with 'public' schema references.

I have ran some tests and found out that only SQL functions cause suchfailures. I believe this is because they are inlined during CREATEINDEX. The attached pg_dump output file recreates the failure, andshows that prefixing it with "public" avoids the failure, and shows thatindexing of a PL/pgSQL function doesn't fail (see SQL comments).

A more general issue with functions is that if you schema-qualify a callto a function, you will still need all object references in the functionto reference the schema where the object is installed if they are not insearch_path, i.e. schema qualifying the object doesn't propagate thatschema qualification into the function body (functions look insearch_path, not necessarily in the schema in which they are defined).

I am not sure what to suggest, but I don't think we can ignore thisproblem.