RE: Extension relocation vs. schema qualification

From: "Verona, Luiz" <luverona(at)amazon(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>
Cc: "keiko713(at)gmail(dot)com" <keiko713(at)gmail(dot)com>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>
Subject: RE: Extension relocation vs. schema qualification
Date: 2021-10-05 09:57:36
Message-ID: 3a89b08b90ac4aef99bce42ba7a23c67@EX13D20EUC002.ant.amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

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=(at)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(at)leadboat(dot)com>
Sent: Tuesday, July 10, 2018 2:43 AM
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: keiko713(at)gmail(dot)com; pgsql-bugs(at)lists(dot)postgresql(dot)org; PostgreSQL-development <pgsql-hackers(at)postgresql(dot)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 confirm the 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(at)(dot) 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(at)(dot) 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);

Attachment Content-Type Size
v1-earthdistance-cube-fix.patch application/octet-stream 2.7 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2021-10-05 12:26:28 Re: Extension relocation vs. schema qualification
Previous Message David Rowley 2021-10-05 07:05:06 Re: BUG #17213: Wrong result from a query involving Merge Semi Join and Memoize

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2021-10-05 09:59:02 Re: plperl: update ppport.h and fix configure version check
Previous Message Jeevan Ladhe 2021-10-05 09:51:11 Re: refactoring basebackup.c