Subject: [PATCH] Allow an extention to be updated without a script

From: Yugo Nagata <[email-masked]>
Date: Mon, 30 Jan 2023 17:36:55 +0900

When we don't need to execute any command to update an extension from one
version to the next, we can specify a list of such updates following
the pattern 'old_version--target_version' into a new option
updates_without_script.  For example, specifying '1.1--1.2, 1.3--1.4'
means updates from version 1.1 to version 1.2, and from version 1.3
to version 1.4 don't need an update script.  User doesn't need to
provide an update script that doesn't contain any command for such
updates.

The updated path is determined based on both the names of update scripts
and the list in updates_without_script.  If an update script is provided,
the script will be executed even if this update is specified in
updates_without_script.
---
 doc/src/sgml/extend.sgml                      |  30 +++-
 src/backend/commands/extension.c              | 161 +++++++++++++++---
 src/test/modules/test_extensions/Makefile     |   3 +-
 .../expected/test_extensions.out              |   6 +
 .../test_extensions/sql/test_extensions.sql   |   8 +
 .../test_extensions/test_ext9--1.0.sql        |   0
 .../test_extensions/test_ext9--2.0--3.0.sql   |   0
 .../modules/test_extensions/test_ext9.control |   5 +
 8 files changed, 182 insertions(+), 31 deletions(-)
 create mode 100644 src/test/modules/test_extensions/test_ext9--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext9--2.0--3.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext9.control

diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..1c4c978264 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -807,6 +807,17 @@ RETURNS anycompatible AS ...
        </para>
       </listitem>
      </varlistentry>
+
+     <varlistentry>
+      <term><varname>updates_without_script</varname> (<type>string</type>)</term>
+      <listitem>
+       <para>
+        A list of updates that do not need update scripts following the pattern
+        <literal><replaceable>old_version</replaceable>--<replaceable>target_version</replaceable></literal>,
+        for example <literal>updates_without_script = '1.1--1.2, 1.3--1.4'</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
     </variablelist>
 
     <para>
@@ -818,8 +829,9 @@ RETURNS anycompatible AS ...
      Secondary control files follow the same format as the primary control
      file.  Any parameters set in a secondary control file override the
      primary control file when installing or updating to that version of
-     the extension.  However, the parameters <varname>directory</varname> and
-     <varname>default_version</varname> cannot be set in a secondary control file.
+     the extension.  However, the parameters <varname>directory</varname>,
+     <varname>default_version</varname>, and <varname>updates_without_script</varname>
+     cannot be set in a secondary control file.
     </para>
 
     <para>
@@ -1092,6 +1104,20 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
      objects, they are automatically dissociated from the extension.
     </para>
 
+    <para>
+     If you don't need to execute any command to update an extension from one
+     version to the next, provide an update script that doesn't contain
+     any command or specify a list of such updates following the pattern
+     <literal><replaceable>old_version</replaceable>--<replaceable>target_version</replaceable></literal>
+     into <varname>updates_without_script</varname>.  For example,
+     <literal>updates_without_script = '1.1--1.2, 1.3--1.4'</literal>
+     means updates from version <literal>1.1</literal> to version <literal>1.2</literal>
+     and from version <literal>1.3</literal> to version <literal>1.4</literal>
+     don't need an update script. Note that even if an update is specified in
+     <varname>updates_without_script</varname>, if a corresponding update script
+     is provided, the update script will be executed.
+    </para>
+
     <para>
      If an extension has secondary control files, the control parameters
      that are used for an update script are those associated with the script's
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index cf1b1ca571..2475f96030 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -90,6 +90,7 @@ 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	   *updates_without_script;	/* updates that don't need a script */
 } ExtensionControlFile;
 
 /*
@@ -98,14 +99,24 @@ typedef struct ExtensionControlFile
 typedef struct ExtensionVersionInfo
 {
 	char	   *name;			/* name of the starting version */
-	List	   *reachable;		/* List of ExtensionVersionInfo's */
+	List	   *reachable;		/* List of ExtensionUpdateStep's */
 	bool		installable;	/* does this version have an install script? */
+	bool		without_script;	/* reachable from the previous without a script? */
 	/* working state for Dijkstra's algorithm: */
 	bool		distance_known; /* is distance from start known yet? */
 	int			distance;		/* current worst-case distance estimate */
 	struct ExtensionVersionInfo *previous;	/* current best predecessor */
 } ExtensionVersionInfo;
 
+/*
+ * Internal data structure for each step in an update path
+ */
+typedef struct ExtensionUpdateStep
+{
+	ExtensionVersionInfo *next_version;		/* the next version */
+	bool	without_script;		/* reachable to the next without a script? */
+} ExtensionUpdateStep;
+
 /* Local functions */
 static List *find_update_path(List *evi_list,
 							  ExtensionVersionInfo *evi_start,
@@ -606,6 +617,27 @@ parse_extension_control_file(ExtensionControlFile *control,
 								item->name)));
 			}
 		}
+		else if (strcmp(item->name, "updates_without_script") == 0)
+		{
+			/* Need a modifiable copy of string */
+			char	   *rawnames = pstrdup(item->value);
+
+			if (version)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
+								item->name)));
+
+			/* Parse string into list of identifiers */
+			if (!SplitIdentifierString(rawnames, ',', &control->updates_without_script))
+			{
+				/* syntax error in name list */
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("parameter \"%s\" must be a list of \"old_version--target_version\"",
+								item->name)));
+			}
+		}
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1092,6 +1124,7 @@ get_ext_ver_info(const char *versionname, List **evi_list)
 	evi->name = pstrdup(versionname);
 	evi->reachable = NIL;
 	evi->installable = false;
+	evi->without_script = false;
 	/* initialize for later application of Dijkstra's algorithm */
 	evi->distance_known = false;
 	evi->distance = INT_MAX;
@@ -1130,10 +1163,44 @@ get_nearest_unprocessed_vertex(List *evi_list)
 	return evi;
 }
 
+/*
+ * Extract version name(s) from a string in 'vername--vername2' format
+ */
+static bool
+extract_version_names(const char *str, char **vername, char **vername2)
+{
+	*vername = pstrdup(str);
+	*vername2 = strstr(*vername, "--");
+	if (*vername2)
+	{
+		**vername2 = '\0';		/* terminate first version */
+		*vername2 += 2;			/* and point to second */
+
+		/* if there's a third --, it's bogus, ignore it */
+		if (strstr(*vername2, "--"))
+			return false;
+	}
+	return true;
+}
+
+/*
+ * Make ExensionUpdateStep data
+ */
+static ExtensionUpdateStep *
+make_ext_update_step(ExtensionVersionInfo *evi, bool without_script)
+{
+	ExtensionUpdateStep *step = (ExtensionUpdateStep *) palloc(sizeof(ExtensionUpdateStep));
+
+	step->next_version = evi;
+	step->without_script = without_script;
+
+	return step;
+}
+
 /*
  * Obtain information about the set of update scripts available for the
  * specified extension.  The result is a List of ExtensionVersionInfo
- * structs, each with a subsidiary list of the ExtensionVersionInfos for
+ * structs, each with a subsidiary list of the ExtensionUpdateSteps for
  * the versions that can be reached in one step from that version.
  */
 static List *
@@ -1144,11 +1211,13 @@ get_ext_ver_list(ExtensionControlFile *control)
 	char	   *location;
 	DIR		   *dir;
 	struct dirent *de;
+	ListCell   *lc;
 
 	location = get_extension_script_directory(control);
 	dir = AllocateDir(location);
 	while ((de = ReadDir(dir, location)) != NULL)
 	{
+		char	   *vernames;
 		char	   *vername;
 		char	   *vername2;
 		ExtensionVersionInfo *evi;
@@ -1165,29 +1234,43 @@ get_ext_ver_list(ExtensionControlFile *control)
 			continue;
 
 		/* extract version name(s) from 'extname--something.sql' filename */
-		vername = pstrdup(de->d_name + extnamelen + 2);
-		*strrchr(vername, '.') = '\0';
-		vername2 = strstr(vername, "--");
+		vernames = pstrdup(de->d_name + extnamelen + 2);
+		*strrchr(vernames, '.') = '\0';
+		if (!extract_version_names(vernames, &vername, &vername2))
+			continue;
+
+		/* Create ExtensionVersionInfos and link them together */
+		evi = get_ext_ver_info(vername, &evi_list);
 		if (!vername2)
 		{
-			/* It's an install, not update, script; record its version name */
-			evi = get_ext_ver_info(vername, &evi_list);
+			/* It's an install, not update, script. */
 			evi->installable = true;
 			continue;
 		}
-		*vername2 = '\0';		/* terminate first version */
-		vername2 += 2;			/* and point to second */
+		evi2 = get_ext_ver_info(vername2, &evi_list);
+		evi->reachable = lappend(evi->reachable, make_ext_update_step(evi2, false));
+	}
+	FreeDir(dir);
 
-		/* if there's a third --, it's bogus, ignore it */
-		if (strstr(vername2, "--"))
+	/*
+	 * Obtain version information from 'old-version--new-version' list in
+	 * updates_without_script option
+	 */
+	foreach (lc, control->updates_without_script)
+	{
+		char	   *vernames = (char *) lfirst(lc);
+		char	   *vername;
+		char	   *vername2;
+		ExtensionVersionInfo *evi;
+		ExtensionVersionInfo *evi2;
+
+		if (!extract_version_names(vernames, &vername, &vername2))
 			continue;
 
-		/* Create ExtensionVersionInfos and link them together */
 		evi = get_ext_ver_info(vername, &evi_list);
 		evi2 = get_ext_ver_info(vername2, &evi_list);
-		evi->reachable = lappend(evi->reachable, evi2);
+		evi->reachable = lappend(evi->reachable, make_ext_update_step(evi2, true));
 	}
-	FreeDir(dir);
 
 	return evi_list;
 }
@@ -1196,8 +1279,9 @@ get_ext_ver_list(ExtensionControlFile *control)
  * Given an initial and final version name, identify the sequence of update
  * scripts that have to be applied to perform that update.
  *
- * Result is a List of names of versions to transition through (the initial
- * version is *not* included).
+ * Result is a List of the ExtensionUpdateSteps to transition through (the
+ * initial version infomration is *not* included).  Returns NIL if no such
+ * path.
  */
 static List *
 identify_update_path(ExtensionControlFile *control,
@@ -1239,8 +1323,9 @@ identify_update_path(ExtensionControlFile *control,
  * been used for this before, and the initialization done by get_ext_ver_info
  * is still good.  Otherwise, reinitialize all transient fields used here.
  *
- * Result is a List of names of versions to transition through (the initial
- * version is *not* included).  Returns NIL if no such path.
+ * Result is a List of the ExtensionUpdateSteps to transition through (the
+ * initial version infomration is *not* included).  Returns NIL if no such
+ * path.
  */
 static List *
 find_update_path(List *evi_list,
@@ -1280,7 +1365,8 @@ find_update_path(List *evi_list,
 			break;				/* found shortest path to target */
 		foreach(lc, evi->reachable)
 		{
-			ExtensionVersionInfo *evi2 = (ExtensionVersionInfo *) lfirst(lc);
+			ExtensionUpdateStep  *step = (ExtensionUpdateStep *) lfirst(lc);
+			ExtensionVersionInfo *evi2 = step->next_version;
 			int			newdist;
 
 			/* if reject_indirect, treat installable versions as unreachable */
@@ -1291,6 +1377,7 @@ find_update_path(List *evi_list,
 			{
 				evi2->distance = newdist;
 				evi2->previous = evi;
+				evi2->without_script = step->without_script;
 			}
 			else if (newdist == evi2->distance &&
 					 evi2->previous != NULL &&
@@ -1305,6 +1392,16 @@ find_update_path(List *evi_list,
 				 * entries get visited.
 				 */
 				evi2->previous = evi;
+				evi2->without_script = step->without_script;
+			}
+			else if (evi == evi2->previous &&
+					 evi->without_script != evi2->without_script)
+			{
+				/*
+				 * If it is reachable both with and without an update script,
+				 * we prefer to use the script.
+				 */
+				evi2->without_script = false;
 			}
 		}
 	}
@@ -1313,10 +1410,10 @@ find_update_path(List *evi_list,
 	if (!evi_target->distance_known)
 		return NIL;
 
-	/* Build and return list of version names representing the update path */
+	/* Build and return list of update steps representing the update path */
 	result = NIL;
 	for (evi = evi_target; evi != evi_start; evi = evi->previous)
-		result = lcons(evi->name, result);
+		result = lcons(make_ext_update_step(evi, evi->without_script), result);
 
 	return result;
 }
@@ -2332,7 +2429,8 @@ pg_extension_update_paths(PG_FUNCTION_ARGS)
 				appendStringInfoString(&pathbuf, evi1->name);
 				foreach(lcv, path)
 				{
-					char	   *versionName = (char *) lfirst(lcv);
+					ExtensionUpdateStep *step = (ExtensionUpdateStep *) lfirst(lcv);
+					char	   *versionName = step->next_version->name;
 
 					appendStringInfoString(&pathbuf, "--");
 					appendStringInfoString(&pathbuf, versionName);
@@ -3047,7 +3145,8 @@ ApplyExtensionUpdates(Oid extensionOid,
 
 	foreach(lcv, updateVersions)
 	{
-		char	   *versionName = (char *) lfirst(lcv);
+		ExtensionUpdateStep *step = (ExtensionUpdateStep *) lfirst(lcv);
+		char	   *versionName = step->next_version->name;
 		ExtensionControlFile *control;
 		char	   *schemaName;
 		Oid			schemaOid;
@@ -3167,12 +3266,18 @@ ApplyExtensionUpdates(Oid extensionOid,
 		InvokeObjectPostAlterHook(ExtensionRelationId, extensionOid, 0);
 
 		/*
-		 * Finally, execute the update script file
+		 * Finally, execute the update script file if we have
 		 */
-		execute_extension_script(extensionOid, control,
-								 oldVersionName, versionName,
-								 requiredSchemas,
-								 schemaName, schemaOid);
+		if (!step->without_script)
+			execute_extension_script(extensionOid, control,
+									 oldVersionName, versionName,
+									 requiredSchemas,
+									 schemaName, schemaOid);
+		/*
+		 * Otherwise, advance the command counter to make the catalog change visible
+		 */
+		else
+			CommandCounterIncrement();
 
 		/*
 		 * Update prior-version name and loop around.  Since
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..e386b11d9d 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -4,12 +4,13 @@ MODULE = test_extensions
 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_ext7 test_ext8 test_ext9 test_ext_cine test_ext_cor \
             test_ext_cyclic1 test_ext_cyclic2 \
             test_ext_evttrig
 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_ext9--1.0.sql test_ext9--2.0--3.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 \
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..f1788d6c1c 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -121,6 +121,12 @@ Objects in extension "test_ext8"
 
 -- dropping it should still work
 drop extension test_ext8;
+-- test updates_without_script
+create extension test_ext9;
+drop extension test_ext9;
+create extension test_ext9 version '1.0';
+alter extension test_ext9 update to '4.0';
+drop extension test_ext9;
 -- Test creation of extension in temporary schema with two-phase commit,
 -- which should not work.  This function wrapper is useful for portability.
 -- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..16e6c9f94a 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -65,6 +65,14 @@ end';
 -- dropping it should still work
 drop extension test_ext8;
 
+-- test updates_without_script
+create extension test_ext9;
+drop extension test_ext9;
+create extension test_ext9 version '1.0';
+alter extension test_ext9 update to '4.0';
+drop extension test_ext9;
+
+
 -- Test creation of extension in temporary schema with two-phase commit,
 -- which should not work.  This function wrapper is useful for portability.
 
diff --git a/src/test/modules/test_extensions/test_ext9--1.0.sql b/src/test/modules/test_extensions/test_ext9--1.0.sql
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/src/test/modules/test_extensions/test_ext9--2.0--3.0.sql b/src/test/modules/test_extensions/test_ext9--2.0--3.0.sql
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/src/test/modules/test_extensions/test_ext9.control b/src/test/modules/test_extensions/test_ext9.control
new file mode 100644
index 0000000000..5c7b77938e
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext9.control
@@ -0,0 +1,5 @@
+comment = 'Test extension 9'
+default_version = '4.0'
+schema = 'public'
+relocatable = false
+updates_without_script = '1.0--2.0, 3.0--4.0'
-- 
2.25.1

--Multipart=_Tue__31_Jan_2023_05_25_02_+0900_xx78gCVqd=kbvlGG--