Change the naming convention for extension files to use double dashes.

This allows us to have an unambiguous rule for deconstructing the names
of script files and secondary control files, without having to forbid
extension and version names from containing any dashes.  We do have to
forbid them from containing double dashes or leading/trailing dashes,
but neither restriction is likely to bother anyone in practice.
Per discussion, this seems like a better solution overall than the
original design.

diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 394aa87900286a1bc289fc9ff6327b89ffa33c27..50924a78f0e79da1c0bc264d0aa692721698dbe9 100644 (file)
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -368,8 +368,8 @@
      installation's SHAREDIR/extension directory.  There
      must also be at least one SQL script file, which follows the
      naming pattern
-     extension-version.sql
-     (for example, foo-1.0.sql for version 1.0 of
+     extension--version.sql
+     (for example, foo--1.0.sql for version 1.0 of
      extension foo).  By default, the script file(s) are also
      placed in the SHAREDIR/extension directory; but the
      control file can specify a different directory for the script file(s).
@@ -378,7 +378,7 @@
     
      The file format for an extension control file is the same as for the
      postgresql.conf file, namely a list of
-     parameter-name = value
+     parameter_name = value
      assignments, one per line.  Blank lines and comments introduced by
      # are allowed.  Be sure to quote any value that is not
      a single word or number.
@@ -477,7 +477,7 @@
      In addition to the primary control file
      extension.control,
      an extension can have secondary control files named in the style
-     extension-version.control.
+     extension--version.control.
      If supplied, these must be located in the script file directory.
      Secondary control files follow the same format as the primary control
      file.  Any parameters set in a secondary control file override the
@@ -671,15 +671,15 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
      dynamically from one version to the next, you should provide
      update scripts that make the necessary changes to go from
      one version to the next.  Update scripts have names following the pattern
-     extension-oldversion-newversion.sql
-     (for example, foo-1.0-1.1.sql contains the commands to modify
+     extension--oldversion--newversion.sql
+     (for example, foo--1.0--1.1.sql contains the commands to modify
      version 1.0 of extension foo into version
      1.1).
     
 
     
      Given that a suitable update script is available, the command
-     ALTER EXTENSION ... UPDATE will update an installed extension
+     ALTER EXTENSION UPDATE will update an installed extension
      to the specified new version.  The update script is run in the same
      environment that CREATE EXTENSION provides for installation
      scripts: in particular, search_path is set up in the same
@@ -712,7 +712,7 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
      class="parameter">old_version option, which causes it to not run the
      normal installation script for the target version, but instead the update
      script named
-     extension-old_version-target_version.sql.
+     extension--old_version--target_version.sql.
      The choice of the dummy version name to use as 
      class="parameter">old_version is up to the extension author, though
      unpackaged is a common convention.  If you have multiple
@@ -723,7 +723,7 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
     
      ALTER EXTENSION is able to execute sequences of update
      script files to achieve a requested update.  For example, if only
-     foo-1.0-1.1.sql and foo-1.1-2.0.sql are
+     foo--1.0--1.1.sql and foo--1.1--2.0.sql are
      available, ALTER EXTENSION will apply them in sequence if an
      update to version 2.0 is requested when 1.0 is
      currently installed.
@@ -734,11 +734,13 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
      of version names: for example, it does not know whether 1.1
      follows 1.0.  It just matches up the available version names
      and follows the path that requires applying the fewest update scripts.
+     (A version name can actually be any string that doesn't contain
+     -- or leading or trailing -.)
     
 
     
      Sometimes it is useful to provide downgrade scripts, for
-     example foo-1.1-1.0.sql to allow reverting the changes
+     example foo--1.1--1.0.sql to allow reverting the changes
      associated with version 1.1.  If you do that, be careful
      of the possibility that a downgrade script might unexpectedly
      get applied because it yields a shorter path.  The risky case is where
@@ -761,7 +763,7 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
     
 
     
-     The script file pair-1.0.sql looks like this:
+     The script file pair--1.0.sql looks like this:
 
 
 CREATE TYPE pair AS ( k text, v text );
@@ -803,7 +805,7 @@ relocatable = true
 
 
 EXTENSION = pair
-DATA = pair-1.0.sql
+DATA = pair--1.0.sql
 
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -860,7 +862,7 @@ include $(PGXS)
 
 MODULES = isbn_issn
 EXTENSION = isbn_issn
-DATA_built = isbn_issn-1.0.sql
+DATA = isbn_issn--1.0.sql
 DOCS = README.isbn_issn
 
 PG_CONFIG = pg_config


diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c

index 99aface408f29a130a80b8d5884a394790d2835a..0661303fea3bf330fc30fc5d7c8655dcb8a73b46 100644 (file)

--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -57,9 +57,6 @@
 bool           creating_extension = false;
 Oid                CurrentExtensionObject = InvalidOid;
 
-/* Character that separates extension & version names in a script filename */
-#define EXT_VERSION_SEP  '-'
-
 /*
  * Internal data structure to hold the results of parsing a control file
  */
@@ -225,9 +222,42 @@ get_extension_schema(Oid ext_oid)
 static void
 check_valid_extension_name(const char *extensionname)
 {
+   int         namelen = strlen(extensionname);
+
    /*
-    * No directory separators (this is sufficient to prevent ".." style
-    * attacks).
+    * Disallow empty names (the parser rejects empty identifiers anyway,
+    * but let's check).
+    */
+   if (namelen == 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not be empty.")));
+
+   /*
+    * No double dashes, since that would make script filenames ambiguous.
+    */
+   if (strstr(extensionname, "--"))
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not contain \"--\".")));
+
+   /*
+    * No leading or trailing dash either.  (We could probably allow this,
+    * but it would require much care in filename parsing and would make
+    * filenames visually if not formally ambiguous.  Since there's no
+    * real-world use case, let's just forbid it.)
+    */
+   if (extensionname[0] == '-' || extensionname[namelen - 1] == '-')
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not begin or end with \"-\".")));
+
+   /*
+    * No directory separators either (this is sufficient to prevent ".."
+    * style attacks).
     */
    if (first_dir_separator(extensionname) != NULL)
        ereport(ERROR,
@@ -239,16 +269,39 @@ check_valid_extension_name(const char *extensionname)
 static void
 check_valid_version_name(const char *versionname)
 {
-   /* No separators --- would risk confusion of install vs update scripts */
-   if (strchr(versionname, EXT_VERSION_SEP))
+   int         namelen = strlen(versionname);
+
+   /*
+    * Disallow empty names (we could possibly allow this, but there seems
+    * little point).
+    */
+   if (namelen == 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension version name: \"%s\"", versionname),
+                errdetail("Version names must not be empty.")));
+
+   /*
+    * No double dashes, since that would make script filenames ambiguous.
+    */
+   if (strstr(versionname, "--"))
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension version name: \"%s\"", versionname),
+                errdetail("Version names must not contain \"--\".")));
+
+   /*
+    * No leading or trailing dash either.
+    */
+   if (versionname[0] == '-' || versionname[namelen - 1] == '-')
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("invalid extension version name: \"%s\"", versionname),
-                errdetail("Version names must not contain the character \"%c\".",
-                          EXT_VERSION_SEP)));
+                errdetail("Version names must not begin or end with \"-\".")));
+
    /*
-    * No directory separators (this is sufficient to prevent ".." style
-    * attacks).
+    * No directory separators either (this is sufficient to prevent ".."
+    * style attacks).
     */
    if (first_dir_separator(versionname) != NULL)
        ereport(ERROR,
@@ -336,8 +389,8 @@ get_extension_aux_control_filename(ExtensionControlFile *control,
    scriptdir = get_extension_script_directory(control);
 
    result = (char *) palloc(MAXPGPATH);
-   snprintf(result, MAXPGPATH, "%s/%s%c%s.control",
-            scriptdir, control->name, EXT_VERSION_SEP, version);
+   snprintf(result, MAXPGPATH, "%s/%s--%s.control",
+            scriptdir, control->name, version);
 
    pfree(scriptdir);
 
@@ -355,12 +408,11 @@ get_extension_script_filename(ExtensionControlFile *control,
 
    result = (char *) palloc(MAXPGPATH);
    if (from_version)
-       snprintf(result, MAXPGPATH, "%s/%s%c%s%c%s.sql",
-                scriptdir, control->name, EXT_VERSION_SEP, from_version,
-                EXT_VERSION_SEP, version);
+       snprintf(result, MAXPGPATH, "%s/%s--%s--%s.sql",
+                scriptdir, control->name, from_version, version);
    else
-       snprintf(result, MAXPGPATH, "%s/%s%c%s.sql",
-                scriptdir, control->name, EXT_VERSION_SEP, version);
+       snprintf(result, MAXPGPATH, "%s/%s--%s.sql",
+                scriptdir, control->name, version);
 
    pfree(scriptdir);
 
@@ -426,7 +478,7 @@ parse_extension_control_file(ExtensionControlFile *control,
            if (version)
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
-                        errmsg("parameter \"%s\" cannot be set in a per-version extension control file",
+                        errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
                                item->name)));
 
            control->directory = pstrdup(item->value);
@@ -436,7 +488,7 @@ parse_extension_control_file(ExtensionControlFile *control,
            if (version)
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
-                        errmsg("parameter \"%s\" cannot be set in a per-version extension control file",
+                        errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
                                item->name)));
 
            control->default_version = pstrdup(item->value);
@@ -907,16 +959,18 @@ get_ext_ver_list(ExtensionControlFile *control)
 
        /* ... matching extension name followed by separator */
        if (strncmp(de->d_name, control->name, extnamelen) != 0 ||
-           de->d_name[extnamelen] != EXT_VERSION_SEP)
+           de->d_name[extnamelen] != '-' ||
+           de->d_name[extnamelen + 1] != '-')
            continue;
 
-       /* extract version names from 'extname-something.sql' filename */
-       vername = pstrdup(de->d_name + extnamelen + 1);
+       /* extract version names from 'extname--something.sql' filename */
+       vername = pstrdup(de->d_name + extnamelen + 2);
        *strrchr(vername, '.') = '\0';
-       vername2 = strchr(vername, EXT_VERSION_SEP);
+       vername2 = strstr(vername, "--");
        if (!vername2)
            continue;           /* it's not an update script */
-       *vername2++ = '\0';
+       *vername2 = '\0';       /* terminate first version */
+       vername2 += 2;          /* and point to second */
 
        /* Create ExtensionVersionInfos and link them together */
        evi = get_ext_ver_info(vername, &evi_list);
@@ -979,6 +1033,20 @@ identify_update_path(ExtensionControlFile *control,
                evi2->distance = newdist;
                evi2->previous = evi;
            }
+           else if (newdist == evi2->distance &&
+                    evi2->previous != NULL &&
+                    strcmp(evi->name, evi2->previous->name) < 0)
+           {
+               /*
+                * Break ties in favor of the version name that comes first
+                * according to strcmp().  This behavior is undocumented and
+                * users shouldn't rely on it.  We do it just to ensure that
+                * if there is a tie, the update path that is chosen does not
+                * depend on random factors like the order in which directory
+                * entries get visited.
+                */
+               evi2->previous = evi;
+           }
        }
    }
 
@@ -1251,7 +1319,7 @@ CreateExtension(CreateExtensionStmt *stmt)
                                        requiredExtensions);
 
    /*
-    * Apply any comment on extension
+    * Apply any control-file comment on extension
     */
    if (control->comment != NULL)
        CreateComments(extensionOid, ExtensionRelationId, 0, control->comment);
@@ -1544,6 +1612,10 @@ pg_available_extensions(PG_FUNCTION_ARGS)
            extname = pstrdup(de->d_name);
            *strrchr(extname, '.') = '\0';
 
+           /* ignore it if it's an auxiliary control file */
+           if (strstr(extname, "--"))
+               continue;
+
            control = read_extension_control_file(extname);
 
            memset(values, 0, sizeof(values));

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 99aface408f29a130a80b8d5884a394790d2835a..0661303fea3bf330fc30fc5d7c8655dcb8a73b46 100644 (file)
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -57,9 +57,6 @@
 bool           creating_extension = false;
 Oid                CurrentExtensionObject = InvalidOid;
 
-/* Character that separates extension & version names in a script filename */
-#define EXT_VERSION_SEP  '-'
-
 /*
  * Internal data structure to hold the results of parsing a control file
  */
@@ -225,9 +222,42 @@ get_extension_schema(Oid ext_oid)
 static void
 check_valid_extension_name(const char *extensionname)
 {
+   int         namelen = strlen(extensionname);
+
    /*
-    * No directory separators (this is sufficient to prevent ".." style
-    * attacks).
+    * Disallow empty names (the parser rejects empty identifiers anyway,
+    * but let's check).
+    */
+   if (namelen == 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not be empty.")));
+
+   /*
+    * No double dashes, since that would make script filenames ambiguous.
+    */
+   if (strstr(extensionname, "--"))
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not contain \"--\".")));
+
+   /*
+    * No leading or trailing dash either.  (We could probably allow this,
+    * but it would require much care in filename parsing and would make
+    * filenames visually if not formally ambiguous.  Since there's no
+    * real-world use case, let's just forbid it.)
+    */
+   if (extensionname[0] == '-' || extensionname[namelen - 1] == '-')
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension name: \"%s\"", extensionname),
+                errdetail("Extension names must not begin or end with \"-\".")));
+
+   /*
+    * No directory separators either (this is sufficient to prevent ".."
+    * style attacks).
     */
    if (first_dir_separator(extensionname) != NULL)
        ereport(ERROR,
@@ -239,16 +269,39 @@ check_valid_extension_name(const char *extensionname)
 static void
 check_valid_version_name(const char *versionname)
 {
-   /* No separators --- would risk confusion of install vs update scripts */
-   if (strchr(versionname, EXT_VERSION_SEP))
+   int         namelen = strlen(versionname);
+
+   /*
+    * Disallow empty names (we could possibly allow this, but there seems
+    * little point).
+    */
+   if (namelen == 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension version name: \"%s\"", versionname),
+                errdetail("Version names must not be empty.")));
+
+   /*
+    * No double dashes, since that would make script filenames ambiguous.
+    */
+   if (strstr(versionname, "--"))
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("invalid extension version name: \"%s\"", versionname),
+                errdetail("Version names must not contain \"--\".")));
+
+   /*
+    * No leading or trailing dash either.
+    */
+   if (versionname[0] == '-' || versionname[namelen - 1] == '-')
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("invalid extension version name: \"%s\"", versionname),
-                errdetail("Version names must not contain the character \"%c\".",
-                          EXT_VERSION_SEP)));
+                errdetail("Version names must not begin or end with \"-\".")));
+
    /*
-    * No directory separators (this is sufficient to prevent ".." style
-    * attacks).
+    * No directory separators either (this is sufficient to prevent ".."
+    * style attacks).
     */
    if (first_dir_separator(versionname) != NULL)
        ereport(ERROR,
@@ -336,8 +389,8 @@ get_extension_aux_control_filename(ExtensionControlFile *control,
    scriptdir = get_extension_script_directory(control);
 
    result = (char *) palloc(MAXPGPATH);
-   snprintf(result, MAXPGPATH, "%s/%s%c%s.control",
-            scriptdir, control->name, EXT_VERSION_SEP, version);
+   snprintf(result, MAXPGPATH, "%s/%s--%s.control",
+            scriptdir, control->name, version);
 
    pfree(scriptdir);
 
@@ -355,12 +408,11 @@ get_extension_script_filename(ExtensionControlFile *control,
 
    result = (char *) palloc(MAXPGPATH);
    if (from_version)
-       snprintf(result, MAXPGPATH, "%s/%s%c%s%c%s.sql",
-                scriptdir, control->name, EXT_VERSION_SEP, from_version,
-                EXT_VERSION_SEP, version);
+       snprintf(result, MAXPGPATH, "%s/%s--%s--%s.sql",
+                scriptdir, control->name, from_version, version);
    else
-       snprintf(result, MAXPGPATH, "%s/%s%c%s.sql",
-                scriptdir, control->name, EXT_VERSION_SEP, version);
+       snprintf(result, MAXPGPATH, "%s/%s--%s.sql",
+                scriptdir, control->name, version);
 
    pfree(scriptdir);
 
@@ -426,7 +478,7 @@ parse_extension_control_file(ExtensionControlFile *control,
            if (version)
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
-                        errmsg("parameter \"%s\" cannot be set in a per-version extension control file",
+                        errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
                                item->name)));
 
            control->directory = pstrdup(item->value);
@@ -436,7 +488,7 @@ parse_extension_control_file(ExtensionControlFile *control,
            if (version)
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
-                        errmsg("parameter \"%s\" cannot be set in a per-version extension control file",
+                        errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
                                item->name)));
 
            control->default_version = pstrdup(item->value);
@@ -907,16 +959,18 @@ get_ext_ver_list(ExtensionControlFile *control)
 
        /* ... matching extension name followed by separator */
        if (strncmp(de->d_name, control->name, extnamelen) != 0 ||
-           de->d_name[extnamelen] != EXT_VERSION_SEP)
+           de->d_name[extnamelen] != '-' ||
+           de->d_name[extnamelen + 1] != '-')
            continue;
 
-       /* extract version names from 'extname-something.sql' filename */
-       vername = pstrdup(de->d_name + extnamelen + 1);
+       /* extract version names from 'extname--something.sql' filename */
+       vername = pstrdup(de->d_name + extnamelen + 2);
        *strrchr(vername, '.') = '\0';
-       vername2 = strchr(vername, EXT_VERSION_SEP);
+       vername2 = strstr(vername, "--");
        if (!vername2)
            continue;           /* it's not an update script */
-       *vername2++ = '\0';
+       *vername2 = '\0';       /* terminate first version */
+       vername2 += 2;          /* and point to second */
 
        /* Create ExtensionVersionInfos and link them together */
        evi = get_ext_ver_info(vername, &evi_list);
@@ -979,6 +1033,20 @@ identify_update_path(ExtensionControlFile *control,
                evi2->distance = newdist;
                evi2->previous = evi;
            }
+           else if (newdist == evi2->distance &&
+                    evi2->previous != NULL &&
+                    strcmp(evi->name, evi2->previous->name) < 0)
+           {
+               /*
+                * Break ties in favor of the version name that comes first
+                * according to strcmp().  This behavior is undocumented and
+                * users shouldn't rely on it.  We do it just to ensure that
+                * if there is a tie, the update path that is chosen does not
+                * depend on random factors like the order in which directory
+                * entries get visited.
+                */
+               evi2->previous = evi;
+           }
        }
    }
 
@@ -1251,7 +1319,7 @@ CreateExtension(CreateExtensionStmt *stmt)
                                        requiredExtensions);
 
    /*
-    * Apply any comment on extension
+    * Apply any control-file comment on extension
     */
    if (control->comment != NULL)
        CreateComments(extensionOid, ExtensionRelationId, 0, control->comment);
@@ -1544,6 +1612,10 @@ pg_available_extensions(PG_FUNCTION_ARGS)
            extname = pstrdup(de->d_name);
            *strrchr(extname, '.') = '\0';
 
+           /* ignore it if it's an auxiliary control file */
+           if (strstr(extname, "--"))
+               continue;
+
            control = read_extension_control_file(extname);
 
            memset(values, 0, sizeof(values));