From 72a5b1fc880481914da2d4233077438dd87840ca Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 20 Mar 2023 18:37:11 -0400 Subject: [PATCH] Add @extschema:name@ and no_relocate options to extensions. @extschema:name@ extends the existing @extschema@ feature so that we can also insert the schema name of some required extension, thus making cross-extension references robust even if they are in different schemas. However, this has the same hazard as @extschema@: if the schema name is embedded literally in an installed object, rather than being looked up once during extension script execution, then it's no longer safe to relocate the other extension to another schema. To deal with that without restricting things unnecessarily, add a "no_relocate" option to extension control files. This allows an extension to specify that it cannot handle relocation of some of its required extensions, even if in themselves those extensions are relocatable. We detect "no_relocate" requests of dependent extensions during ALTER EXTENSION SET SCHEMA. Regina Obe, reviewed by Sandro Santilli and myself Discussion: https://postgr.es/m/003001d8f4ae$402282c0$c0678840$@pcorp.us --- doc/src/sgml/extend.sgml | 45 ++++++++++- src/backend/commands/extension.c | 81 ++++++++++++++++++- src/test/modules/test_extensions/Makefile | 9 ++- .../expected/test_extensions.out | 77 ++++++++++++++++++ src/test/modules/test_extensions/meson.build | 6 ++ .../test_extensions/sql/test_extensions.sql | 23 ++++++ .../test_ext_req_schema1--1.0.sql | 6 ++ .../test_ext_req_schema1.control | 3 + .../test_ext_req_schema2--1.0.sql | 9 +++ .../test_ext_req_schema2.control | 4 + .../test_ext_req_schema3--1.0.sql | 12 +++ .../test_ext_req_schema3.control | 5 ++ 12 files changed, 272 insertions(+), 8 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.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..218940ee5c 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -739,6 +739,21 @@ RETURNS anycompatible AS ... + + no_relocate (string) + + + A list of names of extensions that this extension depends on that + should be barred from changing their schemas via ALTER + EXTENSION ... SET SCHEMA. + This is needed if this extension's script references the name + of a required extension's schema (using + the @extschema:name@ + syntax) in a way that cannot track renames. + + + + superuser (boolean) @@ -902,8 +917,9 @@ RETURNS anycompatible AS ... For such an extension, set relocatable = false in its control file, and use @extschema@ to refer to the target schema in the script file. All occurrences of this string will be - replaced by the actual target schema's name before the script is - executed. The user can set the target schema using the + replaced by the actual target schema's name (double-quoted if + necessary) before the script is executed. The user can set the + target schema using the SCHEMA option of CREATE EXTENSION. @@ -916,7 +932,7 @@ RETURNS anycompatible AS ... will prevent use of the SCHEMA option of CREATE EXTENSION, unless it specifies the same schema named in the control file. This choice is typically necessary if the extension contains - internal assumptions about schema names that can't be replaced by + internal assumptions about its schema name that can't be replaced by uses of @extschema@. The @extschema@ substitution mechanism is available in this case too, although it is of limited use since the schema name is determined by the control file. @@ -969,6 +985,29 @@ SET LOCAL search_path TO @extschema@, pg_temp; setting of search_path during creation of dependent extensions. + + + If an extension references objects belonging to another extension, + it is recommended to schema-qualify those references. To do that, + write @extschema:name@ + in the extension's script file, where name + is the name of the other extension (which must be listed in this + extension's requires list). This string will be + replaced by the name (double-quoted if necessary) of that extension's + target schema. + Although this notation avoids the need to make hard-wired assumptions + about schema names in the extension's script file, its use may embed + the other extension's schema name into the installed objects of this + extension. (Typically, that happens + when @extschema:name@ is + used inside a string literal, such as a function body or + a search_path setting. In other cases, the object + reference is reduced to an OID during parsing and does not require + subsequent lookups.) If the other extension's schema name is so + embedded, you should prevent the other extension from being relocated + after yours is installed, by adding the name of the other extension to + this one's no_relocate list. + diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 02ff4a9a7f..0eabe18335 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -90,6 +90,8 @@ typedef struct ExtensionControlFile bool trusted; /* allow becoming superuser on the fly? */ int encoding; /* encoding of the script file, or -1 */ List *requires; /* names of prerequisite extensions */ + List *no_relocate; /* names of prerequisite extensions that + * should not be relocated */ } ExtensionControlFile; /* @@ -606,6 +608,21 @@ parse_extension_control_file(ExtensionControlFile *control, item->name))); } } + else if (strcmp(item->name, "no_relocate") == 0) + { + /* Need a modifiable copy of string */ + char *rawnames = pstrdup(item->value); + + /* Parse string into list of identifiers */ + if (!SplitIdentifierString(rawnames, ',', &control->no_relocate)) + { + /* syntax error in name list */ + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("parameter \"%s\" must be a list of extension names", + item->name))); + } + } else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -845,6 +862,8 @@ extension_is_trusted(ExtensionControlFile *control) * Execute the appropriate script file for installing or updating the extension * * If from_version isn't NULL, it's an update + * + * Note: requiredSchemas must be one-for-one with the control->requires list */ static void execute_extension_script(Oid extensionOid, ExtensionControlFile *control, @@ -860,6 +879,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control, int save_nestlevel; StringInfoData pathbuf; ListCell *lc; + ListCell *lc2; /* * Enforce superuser-ness if appropriate. We postpone these checks until @@ -1030,6 +1050,27 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control, CStringGetTextDatum(qSchemaName)); } + /* + * Likewise, substitute required extensions' schema names for + * occurrences of @extschema:extension_name@. + */ + Assert(list_length(control->requires) == list_length(requiredSchemas)); + forboth(lc, control->requires, lc2, requiredSchemas) + { + char *reqextname = (char *) lfirst(lc); + Oid reqschema = lfirst_oid(lc2); + char *schemaName = get_namespace_name(reqschema); + const char *qSchemaName = quote_identifier(schemaName); + char *repltoken; + + repltoken = psprintf("@extschema:%s@", reqextname); + t_sql = DirectFunctionCall3Coll(replace_text, + C_COLLATION_OID, + t_sql, + CStringGetTextDatum(repltoken), + CStringGetTextDatum(qSchemaName)); + } + /* * If module_pathname was set in the control file, substitute its * value for occurrences of MODULE_PATHNAME. @@ -2817,9 +2858,43 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o Oid dep_oldNspOid; /* - * Ignore non-membership dependencies. (Currently, the only other - * case we could see here is a normal dependency from another - * extension.) + * If a dependent extension has a no_relocate request for this + * extension, disallow SET SCHEMA. (XXX it's a bit ugly to do this in + * the same loop that's actually executing the renames: we may detect + * the error condition only after having expended a fair amount of + * work. However, the alternative is to do two scans of pg_depend, + * which seems like optimizing for failure cases. The rename work + * will all roll back cleanly enough if we do fail here.) + */ + if (pg_depend->deptype == DEPENDENCY_NORMAL && + pg_depend->classid == ExtensionRelationId) + { + char *depextname = get_extension_name(pg_depend->objid); + ExtensionControlFile *dcontrol; + ListCell *lc; + + dcontrol = read_extension_control_file(depextname); + foreach(lc, dcontrol->no_relocate) + { + char *nrextname = (char *) lfirst(lc); + + if (strcmp(nrextname, NameStr(extForm->extname)) == 0) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot SET SCHEMA of extension \"%s\" because other extensions prevent it", + NameStr(extForm->extname)), + errdetail("Extension \"%s\" requests no relocation of extension \"%s\".", + depextname, + NameStr(extForm->extname)))); + } + } + } + + /* + * Otherwise, 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) continue; diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile index c3139ab0fc..70fc0c8e66 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_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..a31775a260 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -312,3 +312,80 @@ Objects in extension "test_ext_cine" table ext_cine_tab3 (9 rows) +-- +-- Test @extschema:extname@ syntax and no_relocate option +-- +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 test_s_dep.dep_req1(); + dep_req1 +---------- + req1 +(1 row) + +SELECT dep_req2(); + dep_req2 +----------- + req1 req2 +(1 row) + +SELECT dep_req3(); + dep_req3 +----------- + req1 req3 +(1 row) + +SELECT dep_req3b(); + dep_req3b +----------------- + req1 req2 req3b +(1 row) + +CREATE SCHEMA test_s_dep2; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- fails +ERROR: cannot SET SCHEMA of extension "test_ext_req_schema1" because other extensions prevent it +DETAIL: Extension "test_ext_req_schema3" requests no relocation of extension "test_ext_req_schema1". +ALTER EXTENSION test_ext_req_schema2 SET SCHEMA test_s_dep; -- allowed +SELECT test_s_dep.dep_req1(); + dep_req1 +---------- + req1 +(1 row) + +SELECT test_s_dep.dep_req2(); + dep_req2 +----------- + req1 req2 +(1 row) + +SELECT dep_req3(); + dep_req3 +----------- + req1 req3 +(1 row) + +SELECT dep_req3b(); -- fails +ERROR: function public.dep_req2() does not exist +LINE 1: SELECT public.dep_req2() || ' req3b' + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +QUERY: SELECT public.dep_req2() || ' req3b' +CONTEXT: SQL function "dep_req3b" during startup +DROP EXTENSION test_ext_req_schema3; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok +SELECT test_s_dep2.dep_req1(); + dep_req1 +---------- + req1 +(1 row) + +SELECT test_s_dep.dep_req2(); + dep_req2 +----------- + req1 req2 +(1 row) + +DROP EXTENSION test_ext_req_schema1 CASCADE; +NOTICE: drop cascades to extension test_ext_req_schema2 diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build index c3af3e1721..29e5bb2fb5 100644 --- a/src/test/modules/test_extensions/meson.build +++ b/src/test/modules/test_extensions/meson.build @@ -30,6 +30,12 @@ test_install_data += files( '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_schema3--1.0.sql', + 'test_ext_req_schema3.control', ) tests += { diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql index 41b6cddf0b..f4947e7da6 100644 --- a/src/test/modules/test_extensions/sql/test_extensions.sql +++ b/src/test/modules/test_extensions/sql/test_extensions.sql @@ -209,3 +209,26 @@ CREATE EXTENSION test_ext_cine; ALTER EXTENSION test_ext_cine UPDATE TO '1.1'; \dx+ test_ext_cine + +-- +-- Test @extschema:extname@ syntax and no_relocate option +-- +CREATE SCHEMA test_s_dep; +CREATE EXTENSION test_ext_req_schema1 SCHEMA test_s_dep; +CREATE EXTENSION test_ext_req_schema3 CASCADE; +SELECT test_s_dep.dep_req1(); +SELECT dep_req2(); +SELECT dep_req3(); +SELECT dep_req3b(); +CREATE SCHEMA test_s_dep2; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- fails +ALTER EXTENSION test_ext_req_schema2 SET SCHEMA test_s_dep; -- allowed +SELECT test_s_dep.dep_req1(); +SELECT test_s_dep.dep_req2(); +SELECT dep_req3(); +SELECT dep_req3b(); -- fails +DROP EXTENSION test_ext_req_schema3; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok +SELECT test_s_dep2.dep_req1(); +SELECT test_s_dep.dep_req2(); +DROP EXTENSION test_ext_req_schema1 CASCADE; 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..d3afdb351d --- /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 FUNCTION dep_req1() RETURNS text +LANGUAGE SQL AS $$ SELECT 'req1' $$; 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..d9a8ab5f8b --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_req_schema1.control @@ -0,0 +1,3 @@ +comment = '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.sql b/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql new file mode 100644 index 0000000000..b684fcfb16 --- /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 + +-- This formulation can handle relocation of the required extension. +CREATE FUNCTION dep_req2() RETURNS text +BEGIN ATOMIC + SELECT @extschema:test_ext_req_schema1@.dep_req1() || ' req2'; +END; 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..0061202dee --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql @@ -0,0 +1,12 @@ +/* src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql */ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_ext_req_schema3" to load this file. \quit + +-- This formulation cannot handle relocation of the required extension. +CREATE FUNCTION dep_req3() RETURNS text +LANGUAGE SQL IMMUTABLE PARALLEL SAFE +AS $$ SELECT @extschema:test_ext_req_schema1@.dep_req1() || ' req3' $$; + +CREATE FUNCTION dep_req3b() RETURNS text +LANGUAGE SQL IMMUTABLE PARALLEL SAFE +AS $$ SELECT @extschema:test_ext_req_schema2@.dep_req2() || ' req3b' $$; 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..2c631cdf46 --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_req_schema3.control @@ -0,0 +1,5 @@ +comment = 'Test schema referencing of 2 required extensions' +default_version = '1.0' +relocatable = true +requires = 'test_ext_req_schema1, test_ext_req_schema2' +no_relocate = 'test_ext_req_schema1'