Thread: BUG #15112: Unable to run pg_upgrade with earthdistance extension

BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      15112
Logged by:          Keiko Oda
Email address:      keiko713@gmail.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.


Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
Bruce Momjian
Date:
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@gmail.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")::doubleprecision, ("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.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +


Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
Bruce Momjian
Date:
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@gmail.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")::doubleprecision, ("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 for
more input.  (If this is not the proper procedure, please let me know.)

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

    SET check_function_bodies = false;

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

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

A more general issue with functions is that if you schema-qualify a call
to a function, you will still need all object references in the function
to reference the schema where the object is installed if they are not in
search_path, i.e. schema qualifying the object doesn't propagate that
schema qualification into the function body (functions look in
search_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 this
problem.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Attachment

Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
Bruce Momjian
Date:
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@gmail.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")::doubleprecision, ("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 for
more input.  (If this is not the proper procedure, please let me know.)

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

    SET check_function_bodies = false;

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

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

A more general issue with functions is that if you schema-qualify a call
to a function, you will still need all object references in the function
to reference the schema where the object is installed if they are not in
search_path, i.e. schema qualifying the object doesn't propagate that
schema qualification into the function body (functions look in
search_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 this
problem.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Attachment

Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
Noah Misch
Date:
On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > 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?  

The task is to convert it to being a non-relocatable extension that uses
@extschema@, like here:
https://www.postgresql.org/docs/devel/static/extend-extensions.html#EXTEND-EXTENSIONS-EXAMPLE

Keiko Oda, you can hack around this by modifying the problematic extension
function in each database:

CREATE OR REPLACE FUNCTION public.ll_to_earth(float8, float8)
RETURNS public.earth
LANGUAGE SQL
IMMUTABLE STRICT
PARALLEL SAFE
AS 'SELECT
public.cube(public.cube(public.cube(public.earth()*cos(radians($1))*cos(radians($2))),public.earth()*cos(radians($1))*sin(radians($2))),public.earth()*sin(radians($1)))::public.earth';

There's no need to drop and recreate indexes.  However, if the table is small
and nothing depends on the index, that is an easy workaround that one can
employ right now.  (Drop before upgrade and re-create after upgrade.)

> > FYI, earthdistance is certainly not the only case of this problem.  

True; I've been expecting a report like this for contrib/xml2 especially.


Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From
Noah Misch
Date:
On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > 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?  

The task is to convert it to being a non-relocatable extension that uses
@extschema@, like here:
https://www.postgresql.org/docs/devel/static/extend-extensions.html#EXTEND-EXTENSIONS-EXAMPLE

Keiko Oda, you can hack around this by modifying the problematic extension
function in each database:

CREATE OR REPLACE FUNCTION public.ll_to_earth(float8, float8)
RETURNS public.earth
LANGUAGE SQL
IMMUTABLE STRICT
PARALLEL SAFE
AS 'SELECT
public.cube(public.cube(public.cube(public.earth()*cos(radians($1))*cos(radians($2))),public.earth()*cos(radians($1))*sin(radians($2))),public.earth()*sin(radians($1)))::public.earth';

There's no need to drop and recreate indexes.  However, if the table is small
and nothing depends on the index, that is an easy workaround that one can
employ right now.  (Drop before upgrade and re-create after upgrade.)

> > FYI, earthdistance is certainly not the only case of this problem.  

True; I've been expecting a report like this for contrib/xml2 especially.


Extension relocation vs. schema qualification

From
Noah Misch
Date:
On Wed, Apr 04, 2018 at 11:59:57PM -0700, Noah Misch wrote:
> On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > > 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?  
> 
> The task is to convert it to being a non-relocatable extension that uses
> @extschema@, like here:
> https://www.postgresql.org/docs/devel/static/extend-extensions.html#EXTEND-EXTENSIONS-EXAMPLE

Extension "earthdistance" creates sql-language functions that call functions
of extension "cube", which may appear outside @extschema@ and may relocate at
any moment.  Hence, this case is not as simple as using @extschema@.  While
the cube/earthdistance system happens to be revealing this problem, it would
arise in many cases of a function referring to an object of a relocatable
extension.  I see these options:

1. Stop using relocatable=true in core extensions (by adding a new version
   number and versioned control file).  To relocate an extension, drop and
   recreate it.  Deprecate relocatable=true.  Sub-options:

   1a. Require that "earthdistance" and "cube" appear in the same schema by
   forcing an error[1] when they don't.

   1b. Expand @DEPNAME_schema@ in extension SQL files.  Use @cube_schema@ to
   refer to the right objects.

   1c. Use plpgsql to query pg_extension.extnamespace, then EXECUTE a CREATE
   FUNCTION statement after substituting the right schema names.

2. Like (1), including all sub-options, but warn about the problem without
   deprecating relocatable=true.  Drop relocatable=true from extensions that
   have cause to do so: cube, earthdistance, pageinspect, pg_freespacemap,
   xml2.  Do likewise for others as needed in the future.

3. Make "earthdistance" dynamically discover the location of "cube" during
   each function call.  This entails rewriting earthdistance sql-language
   functions in C.  (One could use plpgsql, but that would add a substantial
   performance loss and a runtime dependency.)

4. Re-implement the earthdistance sql functions in C, not calling "cube"
   functions at all.

5. Create copies in "earthdistance" of the "cube" functions it uses[2].  This
   violates modularity.  It makes \dx+ uglier.

6. Allow an extension to ship SQL commands for re-binding to schemas when it
   or a dependency relocates.  This would allow relocatable=true in extensions
   that refer to @extschema@.  Include (1b) in this project.

7. Augment function system and LANGUAGE sql to offer the ability to parse at
   CREATE time, storing a Query tree like we do for views/rules.  (This would
   be a complex feature.)  (One can simulate this today with a rule[3], but it
   defeats inline_function().)

Overall, I lean toward (2b).  It's a self-contained project that doesn't
uglify contrib and that sets a reasonable example for non-core extensions.
While (7) would solve this and other problems nicely, it's a poor back-patch
candidate.  I liked (1b) for awhile, but it would be overkill if we ever get
(7).  Other ideas or preferences?

Thanks,
nm

[1] SELECT 'cube and earthdistance must appear in the same schema',
    1 / (count(DISTINCT extnamespace) = 1)::int
    FROM pg_extension WHERE extname IN ('cube', 'earthdistance');

[2] CREATE FUNCTION _earthdistance_cube(float8) RETURNS cube
    AS '$libdir/cube', 'cube_f8'
    LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

[3] CREATE TABLE sum_scratch (addend0 int, addend1 int, sum int);
    CREATE VIEW sum_impl AS SELECT * FROM sum_scratch;
    CREATE RULE sum_calc AS
     ON INSERT TO sum_impl DO INSTEAD INSERT INTO sum_scratch
     VALUES (NEW.addend0, NEW.addend1, NEW.addend0 + NEW.addend1) RETURNING *;
    CREATE FUNCTION sum(int, int) RETURNS int LANGUAGE sql AS
     $$INSERT INTO sum_impl VALUES ($1, $2) RETURNING sum$$;
    SELECT sum(4, 3);


Extension relocation vs. schema qualification

From
Noah Misch
Date:
On Wed, Apr 04, 2018 at 11:59:57PM -0700, Noah Misch wrote:
> On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > > 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?  
> 
> The task is to convert it to being a non-relocatable extension that uses
> @extschema@, like here:
> https://www.postgresql.org/docs/devel/static/extend-extensions.html#EXTEND-EXTENSIONS-EXAMPLE

Extension "earthdistance" creates sql-language functions that call functions
of extension "cube", which may appear outside @extschema@ and may relocate at
any moment.  Hence, this case is not as simple as using @extschema@.  While
the cube/earthdistance system happens to be revealing this problem, it would
arise in many cases of a function referring to an object of a relocatable
extension.  I see these options:

1. Stop using relocatable=true in core extensions (by adding a new version
   number and versioned control file).  To relocate an extension, drop and
   recreate it.  Deprecate relocatable=true.  Sub-options:

   1a. Require that "earthdistance" and "cube" appear in the same schema by
   forcing an error[1] when they don't.

   1b. Expand @DEPNAME_schema@ in extension SQL files.  Use @cube_schema@ to
   refer to the right objects.

   1c. Use plpgsql to query pg_extension.extnamespace, then EXECUTE a CREATE
   FUNCTION statement after substituting the right schema names.

2. Like (1), including all sub-options, but warn about the problem without
   deprecating relocatable=true.  Drop relocatable=true from extensions that
   have cause to do so: cube, earthdistance, pageinspect, pg_freespacemap,
   xml2.  Do likewise for others as needed in the future.

3. Make "earthdistance" dynamically discover the location of "cube" during
   each function call.  This entails rewriting earthdistance sql-language
   functions in C.  (One could use plpgsql, but that would add a substantial
   performance loss and a runtime dependency.)

4. Re-implement the earthdistance sql functions in C, not calling "cube"
   functions at all.

5. Create copies in "earthdistance" of the "cube" functions it uses[2].  This
   violates modularity.  It makes \dx+ uglier.

6. Allow an extension to ship SQL commands for re-binding to schemas when it
   or a dependency relocates.  This would allow relocatable=true in extensions
   that refer to @extschema@.  Include (1b) in this project.

7. Augment function system and LANGUAGE sql to offer the ability to parse at
   CREATE time, storing a Query tree like we do for views/rules.  (This would
   be a complex feature.)  (One can simulate this today with a rule[3], but it
   defeats inline_function().)

Overall, I lean toward (2b).  It's a self-contained project that doesn't
uglify contrib and that sets a reasonable example for non-core extensions.
While (7) would solve this and other problems nicely, it's a poor back-patch
candidate.  I liked (1b) for awhile, but it would be overkill if we ever get
(7).  Other ideas or preferences?

Thanks,
nm

[1] SELECT 'cube and earthdistance must appear in the same schema',
    1 / (count(DISTINCT extnamespace) = 1)::int
    FROM pg_extension WHERE extname IN ('cube', 'earthdistance');

[2] CREATE FUNCTION _earthdistance_cube(float8) RETURNS cube
    AS '$libdir/cube', 'cube_f8'
    LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

[3] CREATE TABLE sum_scratch (addend0 int, addend1 int, sum int);
    CREATE VIEW sum_impl AS SELECT * FROM sum_scratch;
    CREATE RULE sum_calc AS
     ON INSERT TO sum_impl DO INSTEAD INSERT INTO sum_scratch
     VALUES (NEW.addend0, NEW.addend1, NEW.addend0 + NEW.addend1) RETURNING *;
    CREATE FUNCTION sum(int, int) RETURNS int LANGUAGE sql AS
     $$INSERT INTO sum_impl VALUES ($1, $2) RETURNING sum$$;
    SELECT sum(4, 3);


RE: Extension relocation vs. schema qualification

From
"Verona, Luiz"
Date:
I am writing to resurrect this 3-year-old thread. Attached is a patch to address earthdistance related failures during
pg_restore.

The proposed patch will:
 - Create a new version of earthdistance (1.2) and make this new version default
 - Change earthdistance relocatable from true to false
 - SET SEARCH_PATH=@extschema@ in earthdistance functions
 - Create validation to restrict earthdistance to be created only in the same schema as cube
 - Change cube extension relocatable from true to false to avoid a relocate of cube which will break earthdistance.
 - Change documentation [1] FROM "It is strongly recommended that earthdistance and cube be installed in the same
schema".TO "it's required  that earthdistance and cube be installed and kept in the same schema" 

[1] - https://www.postgresql.org/docs/current/earthdistance.html#EARTHDISTANCE


----------
## Proposal - pg_restore test:

$ psql <<EOF
> select version();
> drop extension if exists cube cascade;
> create extension earthdistance cascade ;
>
> drop schema if exists test cascade;
> create schema test;
>
> --set search_path=test,public;
> drop table if exists test.addresses cascade;
> create table test.addresses (
>       latitude float8,
>       longitude float8
>     );
>
> CREATE INDEX locations ON test.addresses USING gist(ll_to_earth(latitude, longitude));
>
> \dx
> EOF
                                                  version
------------------------------------------------------------------------------------------------------------
 PostgreSQL 15devel on x86_64-pc-linux-gnu, compiled by gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-12), 64-bit
(1 row)

NOTICE:  drop cascades to 2 other objects
DETAIL:  drop cascades to extension earthdistance
drop cascades to index test.locations
DROP EXTENSION
NOTICE:  installing required extension "cube"
CREATE EXTENSION
NOTICE:  drop cascades to table test.addresses
DROP SCHEMA
CREATE SCHEMA
NOTICE:  table "addresses" does not exist, skipping
DROP TABLE
CREATE TABLE
CREATE INDEX
                                    List of installed extensions
     Name      | Version |   Schema   |                         Description
---------------+---------+------------+--------------------------------------------------------------
 cube          | 1.5     | public     | data type for multidimensional cubes
 earthdistance | 1.2     | public     | calculate great-circle distances on the surface of the Earth
 plpgsql       | 1.0     | pg_catalog | PL/pgSQL procedural language
(3 rows)

$ ## dump the schema
$ rm /tmp/test.dmp
$ pg_dump --schema test --format=custom --file /tmp/test.dmp
$
$ ## drop the schema
$ psql <<EOF
> drop schema test cascade ;
> EOF
NOTICE:  drop cascades to table test.addresses
DROP SCHEMA
$
$
$ ## attempt to restore the schema
$ pg_restore -d postgres /tmp/test.dmp
$ ## Check restored schema.
$ psql <<EOF
> \d+ test.addresses
> EOF
                                               Table "test.addresses"
  Column   |       Type       | Collation | Nullable | Default | Storage | Compression | Stats target | Description
-----------+------------------+-----------+----------+---------+---------+-------------+--------------+-------------
 latitude  | double precision |           |          |         | plain   |             |              |
 longitude | double precision |           |          |         | plain   |             |              |
Indexes:
    "locations" gist (ll_to_earth(latitude, longitude))
Access method: heap

----------
## Proposal - schema dependency validation test:
$ psql <<EOF
> select version();
> drop schema if exists test cascade;
> create schema test;
> drop extension if exists cube cascade;
> create extension cube;
> create extension earthdistance with schema test;
>
> \dx
> EOF
                                                  version
------------------------------------------------------------------------------------------------------------
 PostgreSQL 15devel on x86_64-pc-linux-gnu, compiled by gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-12), 64-bit
(1 row)

DROP SCHEMA
CREATE SCHEMA
DROP EXTENSION
CREATE EXTENSION
ERROR:  earthdistance extension must be installed in the same schema as the cube extension
CONTEXT:  PL/pgSQL function inline_code_block line 11 at RAISE
                     List of installed extensions
  Name   | Version |   Schema   |             Description
---------+---------+------------+--------------------------------------
 cube    | 1.5     | public     | data type for multidimensional cubes
 plpgsql | 1.0     | pg_catalog | PL/pgSQL procedural language
(2 rows)






Luiz Verona

-----Original Message-----
From: Noah Misch <noah@leadboat.com>
Sent: Tuesday, July 10, 2018 2:43 AM
To: Bruce Momjian <bruce@momjian.us>
Cc: keiko713@gmail.com; pgsql-bugs@lists.postgresql.org; PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: [EXTERNAL] Extension relocation vs. schema qualification

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can
confirmthe sender and know the content is safe. 



On Wed, Apr 04, 2018 at 11:59:57PM -0700, Noah Misch wrote:
> On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > > 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?
>
> The task is to convert it to being a non-relocatable extension that
> uses @extschema@, like here:
> https://www.postgresql.org/docs/devel/static/extend-extensions.html#EX
> TEND-EXTENSIONS-EXAMPLE

Extension "earthdistance" creates sql-language functions that call functions of extension "cube", which may appear
outside@extschema@ and may relocate at any moment.  Hence, this case is not as simple as using @extschema@.  While the
cube/earthdistancesystem happens to be revealing this problem, it would arise in many cases of a function referring to
anobject of a relocatable extension.  I see these options: 

1. Stop using relocatable=true in core extensions (by adding a new version
   number and versioned control file).  To relocate an extension, drop and
   recreate it.  Deprecate relocatable=true.  Sub-options:

   1a. Require that "earthdistance" and "cube" appear in the same schema by
   forcing an error[1] when they don't.

   1b. Expand @DEPNAME_schema@ in extension SQL files.  Use @cube_schema@ to
   refer to the right objects.

   1c. Use plpgsql to query pg_extension.extnamespace, then EXECUTE a CREATE
   FUNCTION statement after substituting the right schema names.

2. Like (1), including all sub-options, but warn about the problem without
   deprecating relocatable=true.  Drop relocatable=true from extensions that
   have cause to do so: cube, earthdistance, pageinspect, pg_freespacemap,
   xml2.  Do likewise for others as needed in the future.

3. Make "earthdistance" dynamically discover the location of "cube" during
   each function call.  This entails rewriting earthdistance sql-language
   functions in C.  (One could use plpgsql, but that would add a substantial
   performance loss and a runtime dependency.)

4. Re-implement the earthdistance sql functions in C, not calling "cube"
   functions at all.

5. Create copies in "earthdistance" of the "cube" functions it uses[2].  This
   violates modularity.  It makes \dx+ uglier.

6. Allow an extension to ship SQL commands for re-binding to schemas when it
   or a dependency relocates.  This would allow relocatable=true in extensions
   that refer to @extschema@.  Include (1b) in this project.

7. Augment function system and LANGUAGE sql to offer the ability to parse at
   CREATE time, storing a Query tree like we do for views/rules.  (This would
   be a complex feature.)  (One can simulate this today with a rule[3], but it
   defeats inline_function().)

Overall, I lean toward (2b).  It's a self-contained project that doesn't uglify contrib and that sets a reasonable
examplefor non-core extensions. 
While (7) would solve this and other problems nicely, it's a poor back-patch candidate.  I liked (1b) for awhile, but
itwould be overkill if we ever get (7).  Other ideas or preferences? 

Thanks,
nm

[1] SELECT 'cube and earthdistance must appear in the same schema',
    1 / (count(DISTINCT extnamespace) = 1)::int
    FROM pg_extension WHERE extname IN ('cube', 'earthdistance');

[2] CREATE FUNCTION _earthdistance_cube(float8) RETURNS cube
    AS '$libdir/cube', 'cube_f8'
    LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

[3] CREATE TABLE sum_scratch (addend0 int, addend1 int, sum int);
    CREATE VIEW sum_impl AS SELECT * FROM sum_scratch;
    CREATE RULE sum_calc AS
     ON INSERT TO sum_impl DO INSTEAD INSERT INTO sum_scratch
     VALUES (NEW.addend0, NEW.addend1, NEW.addend0 + NEW.addend1) RETURNING *;
    CREATE FUNCTION sum(int, int) RETURNS int LANGUAGE sql AS
     $$INSERT INTO sum_impl VALUES ($1, $2) RETURNING sum$$;
    SELECT sum(4, 3);




Attachment

RE: Extension relocation vs. schema qualification

From
"Verona, Luiz"
Date:
I am writing to resurrect this 3-year-old thread. Attached is a patch to address earthdistance related failures during
pg_restore.

The proposed patch will:
 - Create a new version of earthdistance (1.2) and make this new version default
 - Change earthdistance relocatable from true to false
 - SET SEARCH_PATH=@extschema@ in earthdistance functions
 - Create validation to restrict earthdistance to be created only in the same schema as cube
 - Change cube extension relocatable from true to false to avoid a relocate of cube which will break earthdistance.
 - Change documentation [1] FROM "It is strongly recommended that earthdistance and cube be installed in the same
schema".TO "it's required  that earthdistance and cube be installed and kept in the same schema" 

[1] - https://www.postgresql.org/docs/current/earthdistance.html#EARTHDISTANCE


----------
## Proposal - pg_restore test:

$ psql <<EOF
> select version();
> drop extension if exists cube cascade;
> create extension earthdistance cascade ;
>
> drop schema if exists test cascade;
> create schema test;
>
> --set search_path=test,public;
> drop table if exists test.addresses cascade;
> create table test.addresses (
>       latitude float8,
>       longitude float8
>     );
>
> CREATE INDEX locations ON test.addresses USING gist(ll_to_earth(latitude, longitude));
>
> \dx
> EOF
                                                  version
------------------------------------------------------------------------------------------------------------
 PostgreSQL 15devel on x86_64-pc-linux-gnu, compiled by gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-12), 64-bit
(1 row)

NOTICE:  drop cascades to 2 other objects
DETAIL:  drop cascades to extension earthdistance
drop cascades to index test.locations
DROP EXTENSION
NOTICE:  installing required extension "cube"
CREATE EXTENSION
NOTICE:  drop cascades to table test.addresses
DROP SCHEMA
CREATE SCHEMA
NOTICE:  table "addresses" does not exist, skipping
DROP TABLE
CREATE TABLE
CREATE INDEX
                                    List of installed extensions
     Name      | Version |   Schema   |                         Description
---------------+---------+------------+--------------------------------------------------------------
 cube          | 1.5     | public     | data type for multidimensional cubes
 earthdistance | 1.2     | public     | calculate great-circle distances on the surface of the Earth
 plpgsql       | 1.0     | pg_catalog | PL/pgSQL procedural language
(3 rows)

$ ## dump the schema
$ rm /tmp/test.dmp
$ pg_dump --schema test --format=custom --file /tmp/test.dmp
$
$ ## drop the schema
$ psql <<EOF
> drop schema test cascade ;
> EOF
NOTICE:  drop cascades to table test.addresses
DROP SCHEMA
$
$
$ ## attempt to restore the schema
$ pg_restore -d postgres /tmp/test.dmp
$ ## Check restored schema.
$ psql <<EOF
> \d+ test.addresses
> EOF
                                               Table "test.addresses"
  Column   |       Type       | Collation | Nullable | Default | Storage | Compression | Stats target | Description
-----------+------------------+-----------+----------+---------+---------+-------------+--------------+-------------
 latitude  | double precision |           |          |         | plain   |             |              |
 longitude | double precision |           |          |         | plain   |             |              |
Indexes:
    "locations" gist (ll_to_earth(latitude, longitude))
Access method: heap

----------
## Proposal - schema dependency validation test:
$ psql <<EOF
> select version();
> drop schema if exists test cascade;
> create schema test;
> drop extension if exists cube cascade;
> create extension cube;
> create extension earthdistance with schema test;
>
> \dx
> EOF
                                                  version
------------------------------------------------------------------------------------------------------------
 PostgreSQL 15devel on x86_64-pc-linux-gnu, compiled by gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-12), 64-bit
(1 row)

DROP SCHEMA
CREATE SCHEMA
DROP EXTENSION
CREATE EXTENSION
ERROR:  earthdistance extension must be installed in the same schema as the cube extension
CONTEXT:  PL/pgSQL function inline_code_block line 11 at RAISE
                     List of installed extensions
  Name   | Version |   Schema   |             Description
---------+---------+------------+--------------------------------------
 cube    | 1.5     | public     | data type for multidimensional cubes
 plpgsql | 1.0     | pg_catalog | PL/pgSQL procedural language
(2 rows)






Luiz Verona

-----Original Message-----
From: Noah Misch <noah@leadboat.com>
Sent: Tuesday, July 10, 2018 2:43 AM
To: Bruce Momjian <bruce@momjian.us>
Cc: keiko713@gmail.com; pgsql-bugs@lists.postgresql.org; PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: [EXTERNAL] Extension relocation vs. schema qualification

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can
confirmthe sender and know the content is safe. 



On Wed, Apr 04, 2018 at 11:59:57PM -0700, Noah Misch wrote:
> On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > > 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?
>
> The task is to convert it to being a non-relocatable extension that
> uses @extschema@, like here:
> https://www.postgresql.org/docs/devel/static/extend-extensions.html#EX
> TEND-EXTENSIONS-EXAMPLE

Extension "earthdistance" creates sql-language functions that call functions of extension "cube", which may appear
outside@extschema@ and may relocate at any moment.  Hence, this case is not as simple as using @extschema@.  While the
cube/earthdistancesystem happens to be revealing this problem, it would arise in many cases of a function referring to
anobject of a relocatable extension.  I see these options: 

1. Stop using relocatable=true in core extensions (by adding a new version
   number and versioned control file).  To relocate an extension, drop and
   recreate it.  Deprecate relocatable=true.  Sub-options:

   1a. Require that "earthdistance" and "cube" appear in the same schema by
   forcing an error[1] when they don't.

   1b. Expand @DEPNAME_schema@ in extension SQL files.  Use @cube_schema@ to
   refer to the right objects.

   1c. Use plpgsql to query pg_extension.extnamespace, then EXECUTE a CREATE
   FUNCTION statement after substituting the right schema names.

2. Like (1), including all sub-options, but warn about the problem without
   deprecating relocatable=true.  Drop relocatable=true from extensions that
   have cause to do so: cube, earthdistance, pageinspect, pg_freespacemap,
   xml2.  Do likewise for others as needed in the future.

3. Make "earthdistance" dynamically discover the location of "cube" during
   each function call.  This entails rewriting earthdistance sql-language
   functions in C.  (One could use plpgsql, but that would add a substantial
   performance loss and a runtime dependency.)

4. Re-implement the earthdistance sql functions in C, not calling "cube"
   functions at all.

5. Create copies in "earthdistance" of the "cube" functions it uses[2].  This
   violates modularity.  It makes \dx+ uglier.

6. Allow an extension to ship SQL commands for re-binding to schemas when it
   or a dependency relocates.  This would allow relocatable=true in extensions
   that refer to @extschema@.  Include (1b) in this project.

7. Augment function system and LANGUAGE sql to offer the ability to parse at
   CREATE time, storing a Query tree like we do for views/rules.  (This would
   be a complex feature.)  (One can simulate this today with a rule[3], but it
   defeats inline_function().)

Overall, I lean toward (2b).  It's a self-contained project that doesn't uglify contrib and that sets a reasonable
examplefor non-core extensions. 
While (7) would solve this and other problems nicely, it's a poor back-patch candidate.  I liked (1b) for awhile, but
itwould be overkill if we ever get (7).  Other ideas or preferences? 

Thanks,
nm

[1] SELECT 'cube and earthdistance must appear in the same schema',
    1 / (count(DISTINCT extnamespace) = 1)::int
    FROM pg_extension WHERE extname IN ('cube', 'earthdistance');

[2] CREATE FUNCTION _earthdistance_cube(float8) RETURNS cube
    AS '$libdir/cube', 'cube_f8'
    LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

[3] CREATE TABLE sum_scratch (addend0 int, addend1 int, sum int);
    CREATE VIEW sum_impl AS SELECT * FROM sum_scratch;
    CREATE RULE sum_calc AS
     ON INSERT TO sum_impl DO INSTEAD INSERT INTO sum_scratch
     VALUES (NEW.addend0, NEW.addend1, NEW.addend0 + NEW.addend1) RETURNING *;
    CREATE FUNCTION sum(int, int) RETURNS int LANGUAGE sql AS
     $$INSERT INTO sum_impl VALUES ($1, $2) RETURNING sum$$;
    SELECT sum(4, 3);