Re: Marking some contrib modules as trusted extensions

Andres Freund <andres@anarazel.de> writes:
> On 2020-02-13 18:57:10 -0500, Tom Lane wrote:
>> Maybe we could decide that the time for supporting easy updates from
>> pre-9.1 is past, and just remove all the unpackaged-to-XXX scripts?
>> Maybe even remove the "FROM version" option altogether.

> Yea, that strikes me as a reasonable thing to do. These days that just
> seems to be dangerous, without much advantage.

Here's a patch to remove the core-code support and documentation for
that.  I have not included the actual deletion of the contrib modules'
'unpackaged' scripts, as that seems both long and boring.

            regards, tom lane

diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index 08bb110..261a559 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -88,22 +88,6 @@ CREATE EXTENSION <replaceable>module_name</replaceable>;
  </para>

  <para>
-  If your database was brought forward by dump and reload from a pre-9.1
-  version of <productname>PostgreSQL</productname>, and you had been using the pre-9.1
-  version of the module in it, you should instead do
-
-<programlisting>
-CREATE EXTENSION <replaceable>module_name</replaceable> FROM unpackaged;
-</programlisting>
-
-  This will update the pre-9.1 objects of the module into a proper
-  <firstterm>extension</firstterm> object.  Future updates to the module will be
-  managed by <xref linkend="sql-alterextension"/>.
-  For more information about extension updates, see
-  <xref linkend="extend-extensions"/>.
- </para>
-
- <para>
   Note, however, that some of these modules are not <quote>extensions</quote>
   in this sense, but are loaded into the server in some other way, for instance
   by way of
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index ffe068b..9ec1af7 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -917,33 +917,6 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
     </para>

     <para>
-     The update mechanism can be used to solve an important special case:
-     converting a <quote>loose</quote> collection of objects into an extension.
-     Before the extension mechanism was added to
-     <productname>PostgreSQL</productname> (in 9.1), many people wrote
-     extension modules that simply created assorted unpackaged objects.
-     Given an existing database containing such objects, how can we convert
-     the objects into a properly packaged extension?  Dropping them and then
-     doing a plain <command>CREATE EXTENSION</command> is one way, but it's not
-     desirable if the objects have dependencies (for example, if there are
-     table columns of a data type created by the extension).  The way to fix
-     this situation is to create an empty extension, then use <command>ALTER
-     EXTENSION ADD</command> to attach each pre-existing object to the extension,
-     then finally create any new objects that are in the current extension
-     version but were not in the unpackaged release.  <command>CREATE
-     EXTENSION</command> supports this case with its <literal>FROM</literal> <replaceable
-     class="parameter">old_version</replaceable> option, which causes it to not run the
-     normal installation script for the target version, but instead the update
-     script named
-
<literal><replaceable>extension</replaceable>--<replaceable>old_version</replaceable>--<replaceable>target_version</replaceable>.sql</literal>.
-     The choice of the dummy version name to use as <replaceable
-     class="parameter">old_version</replaceable> is up to the extension author, though
-     <literal>unpackaged</literal> is a common convention.  If you have multiple
-     prior versions you need to be able to update into extension style, use
-     multiple dummy version names to identify them.
-    </para>
-
-    <para>
      <command>ALTER EXTENSION</command> is able to execute sequences of update
      script files to achieve a requested update.  For example, if only
      <literal>foo--1.0--1.1.sql</literal> and <literal>foo--1.1--2.0.sql</literal> are
diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index d76ac3e..6a21bff 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -24,7 +24,6 @@ PostgreSQL documentation
 CREATE EXTENSION [ IF NOT EXISTS ] <replaceable class="parameter">extension_name</replaceable>
     [ WITH ] [ SCHEMA <replaceable class="parameter">schema_name</replaceable> ]
              [ VERSION <replaceable class="parameter">version</replaceable> ]
-             [ FROM <replaceable class="parameter">old_version</replaceable> ]
              [ CASCADE ]
 </synopsis>
  </refsynopsisdiv>
@@ -48,8 +47,9 @@ CREATE EXTENSION [ IF NOT EXISTS ] <replaceable class="parameter">extension_name

   <para>
    The user who runs <command>CREATE EXTENSION</command> becomes the
-   owner of the extension for purposes of later privilege checks, as well
-   as the owner of any objects created by the extension's script.
+   owner of the extension for purposes of later privilege checks, and
+   normally also becomes the owner of any objects created by the
+   extension's script.
   </para>

   <para>
@@ -142,33 +142,6 @@ CREATE EXTENSION [ IF NOT EXISTS ] <replaceable class="parameter">extension_name
      </varlistentry>

      <varlistentry>
-      <term><replaceable class="parameter">old_version</replaceable></term>
-      <listitem>
-       <para>
-        <literal>FROM</literal> <replaceable class="parameter">old_version</replaceable>
-        must be specified when, and only when, you are attempting to install
-        an extension that replaces an <quote>old style</quote> module that is just
-        a collection of objects not packaged into an extension.  This option
-        causes <command>CREATE EXTENSION</command> to run an alternative installation
-        script that absorbs the existing objects into the extension, instead
-        of creating new objects.  Be careful that <literal>SCHEMA</literal> specifies
-        the schema containing these pre-existing objects.
-       </para>
-
-       <para>
-        The value to use for <replaceable
-        class="parameter">old_version</replaceable> is determined by the
-        extension's author, and might vary if there is more than one version
-        of the old-style module that can be upgraded into an extension.
-        For the standard additional modules supplied with pre-9.1
-        <productname>PostgreSQL</productname>, use <literal>unpackaged</literal>
-        for <replaceable class="parameter">old_version</replaceable> when
-        updating a module to extension style.
-       </para>
-      </listitem>
-     </varlistentry>
-
-     <varlistentry>
       <term><literal>CASCADE</literal></term>
       <listitem>
        <para>
@@ -220,16 +193,6 @@ CREATE EXTENSION [ IF NOT EXISTS ] <replaceable class="parameter">extension_name
 CREATE EXTENSION hstore;
 </programlisting>
   </para>
-
-  <para>
-   Update a pre-9.1 installation of <literal>hstore</literal> into
-   extension style:
-<programlisting>
-CREATE EXTENSION hstore SCHEMA public FROM unpackaged;
-</programlisting>
-   Be careful to specify the schema in which you installed the existing
-   <literal>hstore</literal> objects.
-  </para>
  </refsect1>

  <refsect1>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index ddd46f4..a0db7db 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1357,7 +1357,6 @@ static ObjectAddress
 CreateExtensionInternal(char *extensionName,
                         char *schemaName,
                         const char *versionName,
-                        const char *oldVersionName,
                         bool cascade,
                         List *parents,
                         bool is_create)
@@ -1367,6 +1366,8 @@ CreateExtensionInternal(char *extensionName,
     Oid            extowner = GetUserId();
     ExtensionControlFile *pcontrol;
     ExtensionControlFile *control;
+    char       *filename;
+    struct stat fst;
     List       *updateVersions;
     List       *requiredExtensions;
     List       *requiredSchemas;
@@ -1401,56 +1402,6 @@ CreateExtensionInternal(char *extensionName,
      * does what is needed, we try to find a sequence of update scripts that
      * will get us there.
      */
-    if (oldVersionName)
-    {
-        /*
-         * "FROM old_version" was specified, indicating that we're trying to
-         * update from some unpackaged version of the extension.  Locate a
-         * series of update scripts that will do it.
-         */
-        check_valid_version_name(oldVersionName);
-
-        if (strcmp(oldVersionName, versionName) == 0)
-            ereport(ERROR,
-                    (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                     errmsg("FROM version must be different from installation target version \"%s\"",
-                            versionName)));
-
-        updateVersions = identify_update_path(pcontrol,
-                                              oldVersionName,
-                                              versionName);
-
-        if (list_length(updateVersions) == 1)
-        {
-            /*
-             * Simple case where there's just one update script to run. We
-             * will not need any follow-on update steps.
-             */
-            Assert(strcmp((char *) linitial(updateVersions), versionName) == 0);
-            updateVersions = NIL;
-        }
-        else
-        {
-            /*
-             * Multi-step sequence.  We treat this as installing the version
-             * that is the target of the first script, followed by successive
-             * updates to the later versions.
-             */
-            versionName = (char *) linitial(updateVersions);
-            updateVersions = list_delete_first(updateVersions);
-        }
-    }
-    else
-    {
-        /*
-         * No FROM, so we're installing from scratch.  If there is an install
-         * script for the desired version, we only need to run that one.
-         */
-        char       *filename;
-        struct stat fst;
-
-        oldVersionName = NULL;
-
         filename = get_extension_script_filename(pcontrol, NULL, versionName);
         if (stat(filename, &fst) == 0)
         {
@@ -1484,7 +1435,6 @@ CreateExtensionInternal(char *extensionName,
             /* Otherwise, install best starting point and then upgrade */
             versionName = evi_start->name;
         }
-    }

     /*
      * Fetch control parameters for installation target version
@@ -1624,7 +1574,7 @@ CreateExtensionInternal(char *extensionName,
      * Execute the installation script file
      */
     execute_extension_script(extensionOid, control,
-                             oldVersionName, versionName,
+                             NULL, versionName,
                              requiredSchemas,
                              schemaName, schemaOid);

@@ -1691,7 +1641,6 @@ get_required_extension(char *reqExtensionName,
             addr = CreateExtensionInternal(reqExtensionName,
                                            origSchemaName,
                                            NULL,
-                                           NULL,
                                            cascade,
                                            cascade_parents,
                                            is_create);
@@ -1719,11 +1668,9 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt)
 {
     DefElem    *d_schema = NULL;
     DefElem    *d_new_version = NULL;
-    DefElem    *d_old_version = NULL;
     DefElem    *d_cascade = NULL;
     char       *schemaName = NULL;
     char       *versionName = NULL;
-    char       *oldVersionName = NULL;
     bool        cascade = false;
     ListCell   *lc;

@@ -1787,16 +1734,6 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt)
             d_new_version = defel;
             versionName = defGetString(d_new_version);
         }
-        else if (strcmp(defel->defname, "old_version") == 0)
-        {
-            if (d_old_version)
-                ereport(ERROR,
-                        (errcode(ERRCODE_SYNTAX_ERROR),
-                         errmsg("conflicting or redundant options"),
-                         parser_errposition(pstate, defel->location)));
-            d_old_version = defel;
-            oldVersionName = defGetString(d_old_version);
-        }
         else if (strcmp(defel->defname, "cascade") == 0)
         {
             if (d_cascade)
@@ -1815,7 +1752,6 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt)
     return CreateExtensionInternal(stmt->extname,
                                    schemaName,
                                    versionName,
-                                   oldVersionName,
                                    cascade,
                                    NIL,
                                    true);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1b0edf5..96e7fdb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4460,7 +4460,7 @@ DropTableSpaceStmt: DROP TABLESPACE name
  *
  *        QUERY:
  *             CREATE EXTENSION extension
- *             [ WITH ] [ SCHEMA schema ] [ VERSION version ] [ FROM oldversion ]
+ *             [ WITH ] [ SCHEMA schema ] [ VERSION version ]
  *
  *****************************************************************************/

@@ -4500,7 +4500,10 @@ create_extension_opt_item:
                 }
             | FROM NonReservedWord_or_Sconst
                 {
-                    $$ = makeDefElem("old_version", (Node *)makeString($2), @1);
+                    ereport(ERROR,
+                            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                             errmsg("CREATE EXTENSION ... FROM is no longer supported"),
+                             parser_errposition(@1)));
                 }
             | CASCADE
                 {