From e5595de03ec6ce60afde980ae05e9353a1501fdf Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 24 Mar 2021 12:36:08 -0400 Subject: [PATCH] Tidy up more loose ends related to configurable TOAST compression. 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 24f0e395ac5892cd12e8914646fe921fac5ba23d, 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 --- doc/src/sgml/config.sgml | 44 +++++----- src/backend/access/common/toast_compression.c | 74 ++++++++--------- src/backend/utils/misc/guc.c | 30 ++++--- src/include/access/toast_compression.h | 81 +++++-------------- src/test/regress/expected/compression.out | 4 +- src/test/regress/expected/compression_1.out | 9 +-- 6 files changed, 107 insertions(+), 135 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 1f0e0fc1fb..d63aebb2ff 100644 --- 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; - - default_toast_compression (string) - - default_toast_compression configuration parameter - - - - - This variable sets the default - TOAST - compression method for columns of newly-created tables. The - CREATE TABLE statement can override this default - by specifying the COMPRESSION column option. - - The supported compression methods are pglz and - (if configured at the time PostgreSQL was - built) lz4. - The default is pglz. - - - - default_tablespace (string) @@ -8150,6 +8128,28 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; + + default_toast_compression (enum) + + default_toast_compression configuration parameter + + + + + This variable sets the default + TOAST + compression method for columns of newly-created tables. The + CREATE TABLE statement can override this default + by specifying the COMPRESSION column option. + + The supported compression methods are pglz and + (if configured at the time PostgreSQL was + built) lz4. + The default is pglz. + + + + temp_tablespaces (string) diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index 645eb03bf0..52dedac263 100644 --- 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 index 8bfaa53541..cc0b9f6ad6 100644 --- 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 index 44b73bd57c..46c2544e31 100644 --- 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 index c2f2e0e230..566a1877ea 100644 --- 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 index 6626f8e927..3990933415 100644 --- 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 -- 2.30.2