Refactor code parsing compression option values (-Z/--compress)

This commit moves the code in charge of deparsing the method and detail
strings fed later to parse_compress_specification() to a common routine,
where the backward-compatible case of only an integer being found (N
= 0 => "none", N > 1 => gzip at level N) is handled.

Note that this has a side-effect for pg_basebackup, as we now attempt to
detect "server-" and "client-" before checking for the integer-only
pre-14 grammar, where values like server-N and client-N (without the
follow-up detail string) are now valid rather than failing because of an
unsupported method name.  Past grammars are still handled the same way,
but these flavors are now authorized, and would now switch to consider N
= 0 as no compression and N > 1 as gzip with the compression level used
as N, with the caller still controlling if the compression method should
be done server-side, client-side or is unspecified.  The documentation
of pg_basebackup is updated to reflect that.

This benefits other code paths that would like to rely on the same logic
as pg_basebackup and pg_receivewal with option values used for
compression specifications, one area discussed lately being pg_dump.

Author: Georgios Kokolatos, Michael Paquier
Discussion: https://postgr.es/m/O4mutIrCES8ZhlXJiMvzsivT7ztAMja2lkdL1LJx6O5f22I2W8PBIeLKz7mDLwxHoibcnRAYJXm1pH4tyUNC4a8eDzLn22a6Pb1S74Niexg=@pm.me

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 9683089d6fbc6b43bebfd053d5c801270c1fe71c..db3ad9cd5eba8781bbe18beb196971e623512e77 100644 (file)
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -416,14 +416,18 @@ PostgreSQL documentation
        </para>
        <para>
         The compression method can be set to <literal>gzip</literal>,
-        <literal>lz4</literal>, <literal>zstd</literal>, or
-        <literal>none</literal> for no compression. A compression detail
-        string can optionally be specified.  If the detail string is an
-        integer, it specifies the compression level.  Otherwise, it should be
-        a comma-separated list of items, each of the form
-        <literal>keyword</literal> or <literal>keyword=value</literal>.
+        <literal>lz4</literal>, <literal>zstd</literal>,
+        <literal>none</literal> for no compression or an integer (no
+        compression if 0, <literal>gzip</literal> if greater than 0).
+        A compression detail string can optionally be specified.
+        If the detail string is an integer, it specifies the compression
+        level.  Otherwise, it should be a comma-separated list of items,
+        each of the form <literal>keyword</literal> or
+        <literal>keyword=value</literal>.
         Currently, the supported keywords are <literal>level</literal>
         and <literal>workers</literal>.
+        The detail string cannot be used when the compression method
+        is specified as a plain integer.
        </para>
        <para>
         If no compression level is specified, the default compression level

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 22836ca01a1e73aa4ce3fcffaf3d35ac7b910ae5..920569e447495e863fd8d056193437e40405e939 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -956,27 +956,12 @@ parse_max_rate(char *src)
  * at a later stage.
  */
 static void
-parse_compress_options(char *option, char **algorithm, char **detail,
-                      CompressionLocation *locationres)
+backup_parse_compress_options(char *option, char **algorithm, char **detail,
+                             CompressionLocation *locationres)
 {
-   char       *sep;
-   char       *endp;
-
    /*
-    * Check whether the compression specification consists of a bare integer.
-    *
-    * If so, for backward compatibility, assume gzip.
+    * Strip off any "client-" or "server-" prefix, calculating the location.
     */
-   (void) strtol(option, &endp, 10);
-   if (*endp == '\0')
-   {
-       *locationres = COMPRESS_LOCATION_UNSPECIFIED;
-       *algorithm = pstrdup("gzip");
-       *detail = pstrdup(option);
-       return;
-   }
-
-   /* Strip off any "client-" or "server-" prefix. */
    if (strncmp(option, "server-", 7) == 0)
    {
        *locationres = COMPRESS_LOCATION_SERVER;
@@ -990,27 +975,8 @@ parse_compress_options(char *option, char **algorithm, char **detail,
    else
        *locationres = COMPRESS_LOCATION_UNSPECIFIED;
 
-   /*
-    * Check whether there is a compression detail following the algorithm
-    * name.
-    */
-   sep = strchr(option, ':');
-   if (sep == NULL)
-   {
-       *algorithm = pstrdup(option);
-       *detail = NULL;
-   }
-   else
-   {
-       char       *alg;
-
-       alg = palloc((sep - option) + 1);
-       memcpy(alg, option, sep - option);
-       alg[sep - option] = '\0';
-
-       *algorithm = alg;
-       *detail = pstrdup(sep + 1);
-   }
+   /* fallback to the common parsing for the algorithm and detail */
+   parse_compress_options(option, algorithm, detail);
 }
 
 /*
@@ -2411,8 +2377,8 @@ main(int argc, char **argv)
                compressloc = COMPRESS_LOCATION_UNSPECIFIED;
                break;
            case 'Z':
-               parse_compress_options(optarg, &compression_algorithm,
-                                      &compression_detail, &compressloc);
+               backup_parse_compress_options(optarg, &compression_algorithm,
+                                             &compression_detail, &compressloc);
                break;
            case 'c':
                if (pg_strcasecmp(optarg, "fast") == 0)

diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 63207ca025e0240c4ade63e9c836d5fa72cc5a16..c7a46b8a2abca4972f74aeea2a21732743300f81 100644 (file)
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -57,8 +57,6 @@ static XLogRecPtr endpos = InvalidXLogRecPtr;
 
 
 static void usage(void);
-static void parse_compress_options(char *option, char **algorithm,
-                                  char **detail);
 static DIR *get_destination_dir(char *dest_folder);
 static void close_destination_dir(DIR *dest_dir, char *dest_folder);
 static XLogRecPtr FindStreamingStart(uint32 *tli);
@@ -109,65 +107,6 @@ usage(void)
    printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL);
 }
 
-/*
- * Basic parsing of a value specified for -Z/--compress
- *
- * The parsing consists of a METHOD:DETAIL string fed later on to a more
- * advanced routine in charge of proper validation checks.  This only extracts
- * METHOD and DETAIL.  If only an integer is found, the method is implied by
- * the value specified.
- */
-static void
-parse_compress_options(char *option, char **algorithm, char **detail)
-{
-   char       *sep;
-   char       *endp;
-   long        result;
-
-   /*
-    * Check whether the compression specification consists of a bare integer.
-    *
-    * For backward-compatibility, assume "none" if the integer found is zero
-    * and "gzip" otherwise.
-    */
-   result = strtol(option, &endp, 10);
-   if (*endp == '\0')
-   {
-       if (result == 0)
-       {
-           *algorithm = pstrdup("none");
-           *detail = NULL;
-       }
-       else
-       {
-           *algorithm = pstrdup("gzip");
-           *detail = pstrdup(option);
-       }
-       return;
-   }
-
-   /*
-    * Check whether there is a compression detail following the algorithm
-    * name.
-    */
-   sep = strchr(option, ':');
-   if (sep == NULL)
-   {
-       *algorithm = pstrdup(option);
-       *detail = NULL;
-   }
-   else
-   {
-       char       *alg;
-
-       alg = palloc((sep - option) + 1);
-       memcpy(alg, option, sep - option);
-       alg[sep - option] = '\0';
-
-       *algorithm = alg;
-       *detail = pstrdup(sep + 1);
-   }
-}
 
 /*
  * Check if the filename looks like a WAL file, letting caller know if this

diff --git a/src/common/compression.c b/src/common/compression.c
index df5b627834d5d8294dcd83ab39541fdaa5ff32ad..7e51a0ccf4afccdc874acdba5c0ff2b0412227c7 100644 (file)
--- a/src/common/compression.c
+++ b/src/common/compression.c
@@ -356,3 +356,66 @@ validate_compress_specification(pg_compress_specification *spec)
 
    return NULL;
 }
+
+#ifdef FRONTEND
+
+/*
+ * Basic parsing of a value specified through a command-line option, commonly
+ * -Z/--compress.
+ *
+ * The parsing consists of a METHOD:DETAIL string fed later to
+ * parse_compress_specification().  This only extracts METHOD and DETAIL.
+ * If only an integer is found, the method is implied by the value specified.
+ */
+void
+parse_compress_options(const char *option, char **algorithm, char **detail)
+{
+   char       *sep;
+   char       *endp;
+   long        result;
+
+   /*
+    * Check whether the compression specification consists of a bare integer.
+    *
+    * For backward-compatibility, assume "none" if the integer found is zero
+    * and "gzip" otherwise.
+    */
+   result = strtol(option, &endp, 10);
+   if (*endp == '\0')
+   {
+       if (result == 0)
+       {
+           *algorithm = pstrdup("none");
+           *detail = NULL;
+       }
+       else
+       {
+           *algorithm = pstrdup("gzip");
+           *detail = pstrdup(option);
+       }
+       return;
+   }
+
+   /*
+    * Check whether there is a compression detail following the algorithm
+    * name.
+    */
+   sep = strchr(option, ':');
+   if (sep == NULL)
+   {
+       *algorithm = pstrdup(option);
+       *detail = NULL;
+   }
+   else
+   {
+       char       *alg;
+
+       alg = palloc((sep - option) + 1);
+       memcpy(alg, option, sep - option);
+       alg[sep - option] = '\0';
+
+       *algorithm = alg;
+       *detail = pstrdup(sep + 1);
+   }
+}
+#endif                         /* FRONTEND */

diff --git a/src/include/common/compression.h b/src/include/common/compression.h
index 5d680058ed76072d9fe072b210b20df00f342cb8..46855b1a3b3a571a42f26383a0221c6f0be7f7cd 100644 (file)
--- a/src/include/common/compression.h
+++ b/src/include/common/compression.h
@@ -33,6 +33,8 @@ typedef struct pg_compress_specification
    char       *parse_error;    /* NULL if parsing was OK, else message */
 } pg_compress_specification;
 
+extern void parse_compress_options(const char *option, char **algorithm,
+                                  char **detail);
 extern bool parse_compress_algorithm(char *name, pg_compress_algorithm *algorithm);
 extern const char *get_compress_algorithm_name(pg_compress_algorithm algorithm);