public inbox for [email protected]
help / color / mirror / Atom feedFrom: Regina Obe <[email protected]>
To: [email protected]
Cc: 'Tom Lane' <[email protected]>
Cc: [email protected]
Subject: RE: Ability to reference other extensions by schema in extension scripts
Date: Mon, 6 Feb 2023 05:19:39 -0500
Message-ID: <[email protected]> (raw)
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<20221215085234.rpjzlm7hmsmbanqv@c19>
> > > Here is first version of my patch using the
> > > @extschema:extensionname@ syntax you proposed.
> > >
> > > This patch includes:
> > > 1) Changes to replace references of @extschema:extensionname@ with
> > > the schema of the required extension
> > > 2) Documentation for the feature
> > > 3) Tests for the feature.
> > >
Attached is a revised version of the original patch. It is revised to
prevent
ALTER EXTENSION .. SET SCHEMA if there is a dependent extension that
references the extension in their scripts using @extschema:extensionname@
It also adds additional tests to verify that new feature.
In going thru the code base, I was tempted to add a new dependency type
instead of using the existing DEPENDENCY_AUTO. I think this would be
cleaner, but I felt that was overstepping the area a bit, since it requires
making changes to dependency.h and dependency.c
My main concern with using DEPENDENCY_AUTO is because it was designed for
cases where an object can be dropped without need for CASCADE. In this
case, we don't want a dependent extension to be dropped if it's required is
dropped. However since there will already exist
a DEPENDENCY_NORMAL between the 2 extensions, I figure we are protected
against that issue already.
The issue I ran into is there doesn't seem to be an easy way of checking if
a pg_depend record is already in place, so I ended up dropping it first with
deleteDependencyRecordsForSpecific so I wouldn't need to check and then
reading it.
The reason for that is during CREATE EXTENSION it would need to create the
dependency.
It would also need to do so with ALTER EXTENSION .. UPDATE, since extension
could later on add it in their upgrade scripts and so there end up being
dupes after many ALTER EXTENSION UPDATE calls.
pg_depends getAutoExtensionsOfObject seemed suited to that check, as is
done in
alter.c ExecAlterObjectDependsStmt
/* Avoid duplicates */
currexts = getAutoExtensionsOfObject(address.classId,
address.objectId);
if (!list_member_oid(currexts, refAddr.objectId))
recordDependencyOn(&address, &refAddr,
DEPENDENCY_AUTO_EXTENSION);
but it is hard-coded to only check DEPENDENCY_AUTO_EXTENSION
Why isn't there a variant getAutoExtensionsOfObject take a DEPENDENCY type
as an option so it would be more useful or is there functionality for that I
missed?
Thanks,
Regina
Attachments:
[application/octet-stream] 0002-Allow-use-of-extschema-reqextname-to-reference.patch (16.3K, ../[email protected]/2-0002-Allow-use-of-extschema-reqextname-to-reference.patch)
download | inline diff:
From 92019e9015d7c6a536a580b92569d67fee2127e7 Mon Sep 17 00:00:00 2001
From: Regina Obe <[email protected]>
Date: Sun, 5 Feb 2023 01:35:29 -0500
Subject: [PATCH 2/2] 1. Changes to extension execute logic to allow
referencing required extension schemas by variable 2. Extension tests both
Makefile and meson.build 3. Documentation of the new feature 4. Add a
DEPENDENCY_AUTO dependency whenever currently created extension references a
required schema by variable name in its scripts 5. Prevent an extension from
being relocated if there is a DEPENDENCY_AUTO depend to it
---
doc/src/sgml/extend.sgml | 17 +++++
src/backend/commands/extension.c | 71 ++++++++++++++++++-
src/test/modules/test_extensions/Makefile | 9 ++-
.../expected/test_extensions.out | 66 +++++++++++++++++
src/test/modules/test_extensions/meson.build | 7 ++
.../test_extensions/sql/test_extensions.sql | 25 +++++++
.../test_ext_req_schema1--1.0.sql | 6 ++
.../test_ext_req_schema1.control | 3 +
.../test_ext_req_schema2--1.0--2.0.sql | 7 ++
.../test_ext_req_schema2--1.0.sql | 9 +++
.../test_ext_req_schema2.control | 4 ++
.../test_ext_req_schema3--1.0.sql | 13 ++++
.../test_ext_req_schema3.control | 4 ++
13 files changed, 238 insertions(+), 3 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema1.control
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2.control
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema3.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index b70cbe83ae..6c93bddb36 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -908,6 +908,23 @@ RETURNS anycompatible AS ...
</para>
</listitem>
+ <listitem>
+ <para>
+ An extension might depend on other extensions.
+ It is useful to schema qualify calls to dependent extension to minimize reliance on search_path.
+ This is critical for cases such as functions used in indexes, materialized views, or check constraints.
+ To reference a required extension's schema, you must first have <literal>requires</literal>
+ variable specifying the list of extensions your extension requires.
+ In your extension sql scripts,
+ you can reference a required extension's schema with syntax
+ <literal>@extschema:reqextname@</literal> where <literal>reqextname</literal>
+ is the name of an extension in your <literal>requires</literal> list.
+ All occurrences of this string will be
+ replaced by the schema the required extension is installed in before the script is
+ executed.
+ </para>
+ </listitem>
+
<listitem>
<para>
If the extension does not support relocation at all, set
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index b1509cc505..0fc87c96d3 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1030,6 +1030,70 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
CStringGetTextDatum(qSchemaName));
}
+ /*
+ * If this extension requires other extensions
+ * Check each required extension to see if it's schema
+ * is referenced by @extschema:reqextname@ syntax
+ */
+ if (control->requires)
+ {
+ ObjectAddresses *addrs;
+ addrs = new_object_addresses();
+ ObjectAddress depender;
+ ObjectAddressSet(depender, ExtensionRelationId, CurrentExtensionObject);
+ foreach(lc, control->requires)
+ {
+ char *curreq = (char *) lfirst(lc);
+ Oid reqext;
+ Oid reqschema;
+ reqext = get_required_extension(curreq,
+ control->name,
+ schemaName,
+ false,
+ NIL,
+ false);
+ reqschema = get_extension_schema(reqext);
+ char *reqname;
+ reqname = get_namespace_name(reqschema);
+ StringInfoData rToken;
+ initStringInfo(&rToken);
+ appendStringInfo(&rToken, "%s%s%s", "@extschema:", curreq, "@");
+ if (strstr(c_sql,rToken.data) != NULL){
+ /*
+ * If the required extension's schema is referenced by variable name,
+ * add a dependency that the current extension
+ * references the schema of the required extension
+ **/
+
+ ObjectAddress referenced;
+ ObjectAddressSet(referenced, ExtensionRelationId, reqext);
+
+ /*
+ * Deleting the dependency if it exists, because it will be added again
+ * and no easy check to determine if the dependency already exists
+ */
+ deleteDependencyRecordsForSpecific(depender.classId, depender.objectId,
+ DEPENDENCY_AUTO,
+ referenced.classId, referenced.objectId);
+
+ add_exact_object_address(&referenced, addrs);
+ /*
+ * Replace each occurence of @extschema:reqextname@
+ * with the required extension's schema
+ */
+ t_sql = DirectFunctionCall3Coll(replace_text,
+ C_COLLATION_OID,
+ t_sql,
+ CStringGetTextDatum(rToken.data),
+ CStringGetTextDatum(quote_identifier(reqname)));
+ }
+ }
+ /* Record all required schemas that have their schema referenced in script by this one
+ * with a DEPENDENCY_AUTO in pg_depend
+ */
+ record_object_address_dependencies(&depender, addrs, DEPENDENCY_AUTO);
+ free_object_addresses(addrs);
+ }
/*
* If module_pathname was set in the control file, substitute its
* value for occurrences of MODULE_PATHNAME.
@@ -2816,12 +2880,17 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
ObjectAddress dep;
Oid dep_oldNspOid;
+ /* If a dependent object scripts
+ * reference the variable schema name of this extension
+ * do not allow relocation */
+ if (pg_depend->deptype == DEPENDENCY_AUTO && pg_depend->classid == ExtensionRelationId )
+ elog(ERROR, "Extension can not be relocated because dependent extension references it's location");
/*
* Ignore non-membership dependencies. (Currently, the only other
* case we could see here is a normal dependency from another
* extension.)
*/
- if (pg_depend->deptype != DEPENDENCY_EXTENSION)
+ if (pg_depend->deptype != DEPENDENCY_EXTENSION )
continue;
dep.classId = pg_depend->classid;
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..c073df963c 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,19 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig \
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
+
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_req_schema1--1.0.sql \
+ test_ext_req_schema2--1.0.sql test_ext_req_schema2--1.0--2.0.sql \
+ test_ext_req_schema3--1.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..fc1f69c098 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,69 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema1 SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema3 CASCADE;
+NOTICE: installing required extension "test_ext_req_schema2"
+SELECT dep_req();
+ dep_req
+---------
+ 1032w
+(1 row)
+
+SELECT dep_req2();
+ dep_req2
+----------
+ 1032w
+(1 row)
+
+SELECT dep_req3();
+ dep_req3
+----------
+ 2032w
+(1 row)
+
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+ count
+-------
+ 3
+(1 row)
+
+ALTER EXTENSION test_ext_req_schema2 UPDATE TO '2.0';
+CREATE SCHEMA test_s_dep2;
+ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2;
+ERROR: Extension can not be relocated because dependent extension references it's location
+SELECT dep_req();
+ dep_req
+---------
+ 1update
+(1 row)
+
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+ count
+-------
+ 3
+(1 row)
+
+DROP EXTENSION test_ext_req_schema3;
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+ count
+-------
+ 1
+(1 row)
+
+DROP EXTENSION test_ext_req_schema2;
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+ count
+-------
+ 0
+(1 row)
+
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index 45597ddc23..7519f6e28e 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -31,6 +31,13 @@ install_data(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_req_schema1--1.0.sql',
+ 'test_ext_req_schema1.control',
+ 'test_ext_req_schema2--1.0.sql',
+ 'test_ext_req_schema2.control',
+ 'test_ext_req_schema2--1.0--2.0.sql',
+ 'test_ext_req_schema3.control',
+ 'test_ext_req_schema3--1.0.sql',
kwargs: contrib_data_args,
)
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..41f8e6739d 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,28 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+CREATE SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema1 SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema3 CASCADE;
+SELECT dep_req();
+SELECT dep_req2();
+SELECT dep_req3();
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+ALTER EXTENSION test_ext_req_schema2 UPDATE TO '2.0';
+CREATE SCHEMA test_s_dep2;
+ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2;
+SELECT dep_req();
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+DROP EXTENSION test_ext_req_schema3;
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
+DROP EXTENSION test_ext_req_schema2;
+SELECT count(*)
+ FROM pg_depend AS d
+WHERE d.deptype='a' AND d.refclassid::regclass::text = 'pg_extension' AND d.classid::regclass::text = 'pg_extension';
\ No newline at end of file
diff --git a/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
new file mode 100644
index 0000000000..462fb52145
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema1" to load this file. \quit
+
+CREATE DOMAIN req AS text
+ CONSTRAINT starts_with_1 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '1');
diff --git a/src/test/modules/test_extensions/test_ext_req_schema1.control b/src/test/modules/test_extensions/test_ext_req_schema1.control
new file mode 100644
index 0000000000..9ea4558a90
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema1.control
@@ -0,0 +1,3 @@
+comment = 'Create required extension to be referenced'
+default_version = '1.0'
+relocatable = true
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql b/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
new file mode 100644
index 0000000000..73a44a25e5
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
@@ -0,0 +1,7 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION dep_req() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1update''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
new file mode 100644
index 0000000000..9fe25d48a1
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
@@ -0,0 +1,9 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+CREATE DOMAIN dreq AS text
+ CONSTRAINT starts_with_2 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '2');
+
+CREATE FUNCTION dep_req() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1032w''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2.control b/src/test/modules/test_extensions/test_ext_req_schema2.control
new file mode 100644
index 0000000000..d2ba5add97
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2.control
@@ -0,0 +1,4 @@
+comment = 'Test schema referencing of required extensions'
+default_version = '1.0'
+relocatable = true
+requires = 'test_ext_req_schema1'
diff --git a/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
new file mode 100644
index 0000000000..bd05669097
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
@@ -0,0 +1,13 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+CREATE DOMAIN req2 AS text
+ CONSTRAINT starts_with_2 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '2');
+
+CREATE FUNCTION dep_req2() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1032w''::@extschema:[email protected]';
+
+CREATE FUNCTION dep_req3() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''2032w''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema3.control b/src/test/modules/test_extensions/test_ext_req_schema3.control
new file mode 100644
index 0000000000..b052fad785
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema3.control
@@ -0,0 +1,4 @@
+comment = 'Test schema referencing of 2 required extensions'
+default_version = '1.0'
+relocatable = true
+requires = 'test_ext_req_schema1,test_ext_req_schema2'
--
2.21.0.windows.1
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: RE: Ability to reference other extensions by schema in extension scripts
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox