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

@@ -739,6 +739,21 @@ RETURNS anycompatible AS ...
       </listitem>
      </varlistentry>
 
+     <varlistentry id="extend-extensions-files-no-relocate">
+      <term><varname>no_relocate</varname> (<type>string</type>)</term>
+      <listitem>
+       <para>
+        A list of names of extensions that this extension depends on that
+        should be barred from changing their schemas via <command>ALTER
+        EXTENSION ... SET SCHEMA</command>.
+        This is needed if this extension's script references the name
+        of a required extension's schema (using
+        the <literal>@extschema:<replaceable>name</replaceable>@</literal>
+        syntax) in a way that cannot track renames.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="extend-extensions-files-superuser">
       <term><varname>superuser</varname> (<type>boolean</type>)</term>
       <listitem>
@@ -902,8 +917,9 @@ RETURNS anycompatible AS ...
        For such an extension, set <literal>relocatable = false</literal> in its
        control file, and use <literal>@extschema@</literal> 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
        <literal>SCHEMA</literal> option of <command>CREATE EXTENSION</command>.
       </para>
      </listitem>
@@ -916,7 +932,7 @@ RETURNS anycompatible AS ...
        will prevent use of the <literal>SCHEMA</literal> option of <command>CREATE
        EXTENSION</command>, 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 <literal>@extschema@</literal>.  The <literal>@extschema@</literal>
        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 <varname>search_path</varname> during creation of dependent
      extensions.
     </para>
+
+    <para>
+     If an extension references objects belonging to another extension,
+     it is recommended to schema-qualify those references.  To do that,
+     write <literal>@extschema:<replaceable>name</replaceable>@</literal>
+     in the extension's script file, where <replaceable>name</replaceable>
+     is the name of the other extension (which must be listed in this
+     extension's <literal>requires</literal> 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 <literal>@extschema:<replaceable>name</replaceable>@</literal> is
+     used inside a string literal, such as a function body or
+     a <varname>search_path</varname> 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 <literal>no_relocate</literal> list.
+    </para>
    </sect2>
 
    <sect2 id="extend-extensions-config-tables">

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;

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
 

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

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 += {

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;

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' $$;

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

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;

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'

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' $$;

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'