Tidy up more loose ends related to configurable TOAST compression.

robertmhaas · robertmhaas · commit e5595de03ec6 · 2021-03-24T12:36:08.000-04:00
Change the default_toast_compression GUC to be an enum rather than a string. Earlier, uncommitted versions of the patch supported using CREATE ACCESS METHOD to add new compression methods to a running system, but that idea was dropped before commit. So, we can simplify the GUC handling as well, which has the nice side effect of improving the error messages. While updating the documentation to reflect the new GUC type, also move it back to the right place in the list. I moved this while revising what became commit 24f0e39, but apparently the intended ordering is "alphabetical" rather than "whatever Robert thinks looks nice." Rejigger things to avoid having access/toast_compression.h depend on utils/guc.h, so that we don't end up with every file that includes it also depending on something largely unrelated. Move a few inline functions back into the C source file partly to help reduce dependencies and partly just to avoid clutter. A few very minor cosmetic fixes. Original patch by Justin Pryzby, but very heavily edited by me, and reverse reviewed by him and also reviewed by by Tom Lane. Discussion: http://postgr.es/m/CA+TgmoYp=GT_ztUCeZg2i4hkHAQv8o=-nVJ1-TKWTG1zQOmOpg@mail.gmail.com
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
@@ -8085,28 +8085,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-default-toast-compression" xreflabel="default_toast_compression">
-      <term><varname>default_toast_compression</varname> (<type>string</type>)
-      <indexterm>
-       <primary><varname>default_toast_compression</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        This variable sets the default
-        <link linkend="storage-toast">TOAST</link>
-        compression method for columns of newly-created tables. The
-        <command>CREATE TABLE</command> statement can override this default
-        by specifying the <literal>COMPRESSION</literal> column option.
-
-        The supported compression methods are <literal>pglz</literal> and
-        (if configured at the time <productname>PostgreSQL</productname> was
-        built) <literal>lz4</literal>.
-        The default is <literal>pglz</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-default-tablespace" xreflabel="default_tablespace">
       <term><varname>default_tablespace</varname> (<type>string</type>)
       <indexterm>
@@ -8150,6 +8128,28 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-default-toast-compression" xreflabel="default_toast_compression">
+      <term><varname>default_toast_compression</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>default_toast_compression</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        This variable sets the default
+        <link linkend="storage-toast">TOAST</link>
+        compression method for columns of newly-created tables. The
+        <command>CREATE TABLE</command> statement can override this default
+        by specifying the <literal>COMPRESSION</literal> column option.
+
+        The supported compression methods are <literal>pglz</literal> and
+        (if configured at the time <productname>PostgreSQL</productname> was
+        built) <literal>lz4</literal>.
+        The default is <literal>pglz</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-temp-tablespaces" xreflabel="temp_tablespaces">
       <term><varname>temp_tablespaces</varname> (<type>string</type>)
       <indexterm>
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
@@ -23,8 +23,15 @@
 #include "fmgr.h"
 #include "utils/builtins.h"
 
-/* Compile-time default */
-char	   *default_toast_compression = DEFAULT_TOAST_COMPRESSION;
+/* GUC */
+int	   default_toast_compression = TOAST_PGLZ_COMPRESSION;
+
+#define NO_LZ4_SUPPORT() \
+	ereport(ERROR, \
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), \
+			 errmsg("unsupported LZ4 compression method"), \
+			 errdetail("This functionality requires the server to be built with lz4 support."), \
+			 errhint("You need to rebuild PostgreSQL using --with-lz4.")))
 
 /*
  * Compress a varlena using PGLZ.
@@ -271,46 +278,41 @@ toast_get_compression_id(struct varlena *attr)
 }
 
 /*
- * Validate a new value for the default_toast_compression GUC.
+ * CompressionNameToMethod - Get compression method from compression name
+ *
+ * Search in the available built-in methods.  If the compression not found
+ * in the built-in methods then return InvalidCompressionMethod.
  */
-bool
-check_default_toast_compression(char **newval, void **extra, GucSource source)
+char
+CompressionNameToMethod(const char *compression)
 {
-	if (**newval == '\0')
+	if (strcmp(compression, "pglz") == 0)
+		return TOAST_PGLZ_COMPRESSION;
+	else if (strcmp(compression, "lz4") == 0)
 	{
-		GUC_check_errdetail("%s cannot be empty.",
-							"default_toast_compression");
-		return false;
+#ifndef USE_LZ4
+		NO_LZ4_SUPPORT();
+#endif
+		return TOAST_LZ4_COMPRESSION;
 	}
 
-	if (strlen(*newval) >= NAMEDATALEN)
-	{
-		GUC_check_errdetail("%s is too long (maximum %d characters).",
-							"default_toast_compression", NAMEDATALEN - 1);
-		return false;
-	}
+	return InvalidCompressionMethod;
+}
 
-	if (!CompressionMethodIsValid(CompressionNameToMethod(*newval)))
+/*
+ * GetCompressionMethodName - Get compression method name
+ */
+const char *
+GetCompressionMethodName(char method)
+{
+	switch (method)
 	{
-		/*
-		 * When source == PGC_S_TEST, don't throw a hard error for a
-		 * nonexistent compression method, only a NOTICE. See comments in
-		 * guc.h.
-		 */
-		if (source == PGC_S_TEST)
-		{
-			ereport(NOTICE,
-					(errcode(ERRCODE_UNDEFINED_OBJECT),
-					 errmsg("compression method \"%s\" does not exist",
-							*newval)));
-		}
-		else
-		{
-			GUC_check_errdetail("Compression method \"%s\" does not exist.",
-								*newval);
-			return false;
-		}
+		case TOAST_PGLZ_COMPRESSION:
+			return "pglz";
+		case TOAST_LZ4_COMPRESSION:
+			return "lz4";
+		default:
+			elog(ERROR, "invalid compression method %c", method);
+			return NULL;		/* keep compiler quiet */
 	}
-
-	return true;
 }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
@@ -509,6 +509,14 @@ static struct config_enum_entry shared_memory_options[] = {
 	{NULL, 0, false}
 };
 
+static struct config_enum_entry default_toast_compression_options[] = {
+	{"pglz", TOAST_PGLZ_COMPRESSION, false},
+#ifdef  USE_LZ4
+	{"lz4", TOAST_LZ4_COMPRESSION, false},
+#endif
+	{NULL, 0, false}
+};
+
 /*
  * Options for enum values stored in other modules
  */
@@ -3933,17 +3941,6 @@ static struct config_string ConfigureNamesString[] =
 		check_default_table_access_method, NULL, NULL
 	},
 
-	{
-		{"default_toast_compression", PGC_USERSET, CLIENT_CONN_STATEMENT,
-			gettext_noop("Sets the default compression for new columns."),
-			NULL,
-			GUC_IS_NAME
-		},
-		&default_toast_compression,
-		DEFAULT_TOAST_COMPRESSION,
-		check_default_toast_compression, NULL, NULL
-	},
-
 	{
 		{"default_tablespace", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the default tablespace to create tables and indexes in."),
@@ -4585,6 +4582,17 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"default_toast_compression", PGC_USERSET, CLIENT_CONN_STATEMENT,
+			gettext_noop("Sets the default compression for new columns."),
+			NULL,
+			GUC_IS_NAME
+		},
+		&default_toast_compression,
+		TOAST_PGLZ_COMPRESSION,
+		default_toast_compression_options, NULL, NULL
+	},
+
 	{
 		{"default_transaction_isolation", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the transaction isolation level of each new transaction."),
diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h
@@ -13,18 +13,26 @@
 #ifndef TOAST_COMPRESSION_H
 #define TOAST_COMPRESSION_H
 
-#include "utils/guc.h"
-
-/* GUCs */
-extern char *default_toast_compression;
-
-/* default compression method if not specified. */
-#define DEFAULT_TOAST_COMPRESSION	"pglz"
+/*
+ * GUC support.
+ *
+ * default_toast_compression is an integer for purposes of the GUC machinery,
+ * but the value is one of the char values defined below, as they appear in
+ * pg_attribute.attcompression, e.g. TOAST_PGLZ_COMPRESSION.
+ */
+extern int default_toast_compression;
 
 /*
  * Built-in compression method-id.  The toast compression header will store
  * this in the first 2 bits of the raw length.  These built-in compression
  * method-id are directly mapped to the built-in compression methods.
+ *
+ * Don't use these values for anything other than understanding the meaning
+ * of the raw bits from a varlena; in particular, if the goal is to identify
+ * a compression method, use the constants TOAST_PGLZ_COMPRESSION, etc.
+ * below. We might someday support more than 4 compression methods, but
+ * we can never have more than 4 values in this enum, because there are
+ * only 2 bits available in the places where this is used.
  */
 typedef enum ToastCompressionId
 {
@@ -39,60 +47,13 @@ typedef enum ToastCompressionId
  */
 #define TOAST_PGLZ_COMPRESSION			'p'
 #define TOAST_LZ4_COMPRESSION			'l'
+#define InvalidCompressionMethod		'\0'
 
-#define InvalidCompressionMethod	'\0'
-#define CompressionMethodIsValid(cm)  ((bool) ((cm) != InvalidCompressionMethod))
-
-#define NO_LZ4_SUPPORT() \
-	ereport(ERROR, \
-			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), \
-			 errmsg("unsupported LZ4 compression method"), \
-			 errdetail("This functionality requires the server to be built with lz4 support."), \
-			 errhint("You need to rebuild PostgreSQL using --with-lz4.")))
+#define CompressionMethodIsValid(cm)  ((cm) != InvalidCompressionMethod)
 
 #define IsStorageCompressible(storage) ((storage) != TYPSTORAGE_PLAIN && \
 										(storage) != TYPSTORAGE_EXTERNAL)
 
-/*
- * GetCompressionMethodName - Get compression method name
- */
-static inline const char *
-GetCompressionMethodName(char method)
-{
-	switch (method)
-	{
-		case TOAST_PGLZ_COMPRESSION:
-			return "pglz";
-		case TOAST_LZ4_COMPRESSION:
-			return "lz4";
-		default:
-			elog(ERROR, "invalid compression method %c", method);
-			return NULL;		/* keep compiler quiet */
-	}
-}
-
-/*
- * CompressionNameToMethod - Get compression method from compression name
- *
- * Search in the available built-in methods.  If the compression not found
- * in the built-in methods then return InvalidCompressionMethod.
- */
-static inline char
-CompressionNameToMethod(char *compression)
-{
-	if (strcmp(compression, "pglz") == 0)
-		return TOAST_PGLZ_COMPRESSION;
-	else if (strcmp(compression, "lz4") == 0)
-	{
-#ifndef USE_LZ4
-		NO_LZ4_SUPPORT();
-#endif
-		return TOAST_LZ4_COMPRESSION;
-	}
-
-	return InvalidCompressionMethod;
-}
-
 /*
  * GetDefaultToastCompression -- get the default toast compression method
  *
@@ -101,7 +62,7 @@ CompressionNameToMethod(char *compression)
 static inline char
 GetDefaultToastCompression(void)
 {
-	return CompressionNameToMethod(default_toast_compression);
+	return (char) default_toast_compression;
 }
 
 /* pglz compression/decompression routines */
@@ -115,8 +76,10 @@ extern struct varlena *lz4_compress_datum(const struct varlena *value);
 extern struct varlena *lz4_decompress_datum(const struct varlena *value);
 extern struct varlena *lz4_decompress_datum_slice(const struct varlena *value,
 												  int32 slicelength);
+
+/* other stuff */
 extern ToastCompressionId toast_get_compression_id(struct varlena *attr);
-extern bool check_default_toast_compression(char **newval, void **extra,
-											GucSource source);
+extern char CompressionNameToMethod(const char *compression);
+extern const char *GetCompressionMethodName(char method);
 
 #endif							/* TOAST_COMPRESSION_H */
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
@@ -234,10 +234,10 @@ DETAIL:  pglz versus lz4
 -- test default_toast_compression GUC
 SET default_toast_compression = '';
 ERROR:  invalid value for parameter "default_toast_compression": ""
-DETAIL:  default_toast_compression cannot be empty.
+HINT:  Available values: pglz, lz4.
 SET default_toast_compression = 'I do not exist compression';
 ERROR:  invalid value for parameter "default_toast_compression": "I do not exist compression"
-DETAIL:  Compression method "I do not exist compression" does not exist.
+HINT:  Available values: pglz, lz4.
 SET default_toast_compression = 'lz4';
 DROP TABLE cmdata2;
 CREATE TABLE cmdata2 (f1 text);
diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out
@@ -227,14 +227,13 @@ DETAIL:  pglz versus lz4
 -- test default_toast_compression GUC
 SET default_toast_compression = '';
 ERROR:  invalid value for parameter "default_toast_compression": ""
-DETAIL:  default_toast_compression cannot be empty.
+HINT:  Available values: pglz.
 SET default_toast_compression = 'I do not exist compression';
 ERROR:  invalid value for parameter "default_toast_compression": "I do not exist compression"
-DETAIL:  Compression method "I do not exist compression" does not exist.
+HINT:  Available values: pglz.
 SET default_toast_compression = 'lz4';
-ERROR:  unsupported LZ4 compression method
-DETAIL:  This functionality requires the server to be built with lz4 support.
-HINT:  You need to rebuild PostgreSQL using --with-lz4.
+ERROR:  invalid value for parameter "default_toast_compression": "lz4"
+HINT:  Available values: pglz.
 DROP TABLE cmdata2;
 CREATE TABLE cmdata2 (f1 text);
 \d+ cmdata2
