pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedExtension relocation vs. schema qualification
4+ messages / 4 participants
[nested] [flat]
* Extension relocation vs. schema qualification
@ 2018-07-10 01:43 Noah Misch <noah@leadboat.com>
2021-10-05 09:57 ` RE: Extension relocation vs. schema qualification Verona, Luiz <luverona@amazon.com>
0 siblings, 1 reply; 4+ messages in thread
From: Noah Misch @ 2018-07-10 01:43 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: keiko713@gmail.com, pgsql-bugs@lists.postgresql.org, PostgreSQL-development <pgsql-hackers@postgresql.org>
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);
^ permalink raw reply [nested|flat] 4+ messages in thread
* RE: Extension relocation vs. schema qualification
2018-07-10 01:43 Extension relocation vs. schema qualification Noah Misch <noah@leadboat.com>
@ 2021-10-05 09:57 ` Verona, Luiz <luverona@amazon.com>
2021-10-05 12:26 ` Re: Extension relocation vs. schema qualification Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Verona, Luiz @ 2021-10-05 09:57 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; Bruce Momjian <bruce@momjian.us>; +Cc: keiko713@gmail.com <keiko713@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>; Imseih (AWS), Sami <simseih@amazon.com>
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 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@. 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);
Attachments:
v1-earthdistance-cube-fix.patch (application/octet-stream, 2.7K) download
diff --git a/contrib/cube/cube.control b/contrib/cube/cube.control
index 50427ec117..8af637c901 100644
--- a/contrib/cube/cube.control
+++ b/contrib/cube/cube.control
@@ -2,5 +2,5 @@
comment = 'data type for multidimensional cubes'
default_version = '1.5'
module_pathname = '$libdir/cube'
-relocatable = true
+relocatable = false
trusted = true
diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile
index f93b7a925a..fa648be831 100644
--- a/contrib/earthdistance/Makefile
+++ b/contrib/earthdistance/Makefile
@@ -3,7 +3,7 @@
MODULES = earthdistance
EXTENSION = earthdistance
-DATA = earthdistance--1.1.sql earthdistance--1.0--1.1.sql
+DATA = earthdistance--1.1.sql earthdistance--1.0--1.1.sql earthdistance--1.1--1.2.sql
PGFILEDESC = "earthdistance - calculate distances on the surface of the Earth"
REGRESS = earthdistance
diff --git a/contrib/earthdistance/earthdistance--1.1--1.2.sql b/contrib/earthdistance/earthdistance--1.1--1.2.sql
new file mode 100644
index 0000000000..64e431b8d8
--- /dev/null
+++ b/contrib/earthdistance/earthdistance--1.1--1.2.sql
@@ -0,0 +1,30 @@
+/* contrib/earthdistance/earthdistance--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION earthdistance UPDATE TO '1.2'" to load this file. \quit
+
+DO LANGUAGE plpgsql
+$$
+DECLARE
+ v_cubenspname TEXT;
+BEGIN
+ SELECT nspname INTO v_cubenspname
+ FROM pg_extension, pg_namespace
+ WHERE pg_extension.extnamespace = pg_namespace.oid
+ AND extname='cube';
+
+ IF v_cubenspname != '@extschema@' THEN
+ RAISE EXCEPTION 'earthdistance extension must be installed in the same schema as the cube extension';
+ END IF;
+END
+$$;
+
+ALTER FUNCTION earth() SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION sec_to_gc(float8) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION gc_to_sec(float8) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION ll_to_earth(float8, float8) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION latitude(earth) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION longitude(earth) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION earth_distance(earth, earth) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION earth_box(earth, float8) SET SEARCH_PATH=@extschema@;
+ALTER FUNCTION geo_distance(point, point) SET SEARCH_PATH=@extschema@;
diff --git a/contrib/earthdistance/earthdistance.control b/contrib/earthdistance/earthdistance.control
index 5816d22cdd..834eca2906 100644
--- a/contrib/earthdistance/earthdistance.control
+++ b/contrib/earthdistance/earthdistance.control
@@ -1,6 +1,6 @@
# earthdistance extension
comment = 'calculate great-circle distances on the surface of the Earth'
-default_version = '1.1'
+default_version = '1.2'
module_pathname = '$libdir/earthdistance'
-relocatable = true
+relocatable = false
requires = 'cube'
* Re: Extension relocation vs. schema qualification
2018-07-10 01:43 Extension relocation vs. schema qualification Noah Misch <noah@leadboat.com>
2021-10-05 09:57 ` RE: Extension relocation vs. schema qualification Verona, Luiz <luverona@amazon.com>
@ 2021-10-05 12:26 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-10-06 14:44 ` Re: Extension relocation vs. schema qualification Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2021-10-05 12:26 UTC (permalink / raw)
To: Verona, Luiz <luverona@amazon.com>; +Cc: pgsql-hackers@lists.postgresql.org
"Verona, Luiz" <luverona@amazon.com> writes:
> 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
That part seems like kind of a nonstarter. People may already rely on it
being relocatable. Also, if cube is still relocatable, it could still
get broken post-installation.
I spent some time awhile ago on fixing this via new-style SQL functions
[1]. That didn't get finished for reasons explained in the thread,
but I think that's probably a more productive direction to go in.
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/3395418.1618352794%40sss.pgh.pa.us
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Extension relocation vs. schema qualification
2018-07-10 01:43 Extension relocation vs. schema qualification Noah Misch <noah@leadboat.com>
2021-10-05 09:57 ` RE: Extension relocation vs. schema qualification Verona, Luiz <luverona@amazon.com>
2021-10-05 12:26 ` Re: Extension relocation vs. schema qualification Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-10-06 14:44 ` Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: Robert Haas @ 2021-10-06 14:44 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Verona, Luiz <luverona@amazon.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Oct 5, 2021 at 8:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I spent some time awhile ago on fixing this via new-style SQL functions
> [1]. That didn't get finished for reasons explained in the thread,
> but I think that's probably a more productive direction to go in.
Hmm, if that'll solve the problem, +1 from me. I'm not a big fan of
giving up on the ability to relocate extensions. I know it's a pain to
support and I'm not sure that we have all the ideas we need to make it
work cleanly, but I don't feel like it should be intrinsically
impossible to get working, and I think users want it.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2021-10-06 14:44 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-07-10 01:43 Extension relocation vs. schema qualification Noah Misch <noah@leadboat.com>
2021-10-05 09:57 ` Verona, Luiz <luverona@amazon.com>
2021-10-05 12:26 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-10-06 14:44 ` Robert Haas <robertmhaas@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox