Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lO7vG-0004yE-Ex for pgsql-hackers@arkaria.postgresql.org; Sun, 21 Mar 2021 23:55:54 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lO7vF-0000Vj-A3 for pgsql-hackers@arkaria.postgresql.org; Sun, 21 Mar 2021 23:55:53 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lO7vE-0000Vb-R5 for pgsql-hackers@lists.postgresql.org; Sun, 21 Mar 2021 23:55:53 +0000 Received: from mail-il1-x12d.google.com ([2607:f8b0:4864:20::12d]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1lO7v9-0001ut-Pu for pgsql-hackers@lists.postgresql.org; Sun, 21 Mar 2021 23:55:51 +0000 Received: by mail-il1-x12d.google.com with SMTP id t14so3418725ilu.3 for ; Sun, 21 Mar 2021 16:55:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telsasoft-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=EG0j8/P6IBog1adNhABbm2zVUlafoEOVKLf9OpJ4t9Y=; b=MfftBWhVPfRDzEg+TSTQnddFAgG8bZTpzorMGcMplIcEJMb4SDFofjb02t6c+yEFOg YBI9q5kS2r3gaeYsyqdVBCNjvinc+QP3WDRG+UJzSWUAiiIeNv+EgJbET7hGi3r+BerL 2KoT6SnXEpHWl7xTqPs2Je5KKJIdqpWjiRj2Qc57M9cL9eRh+R/GII/1YnFuWERI30lp T3YbLrM+3jZcoiveSWUWOVAHxbpsF8ubMIB5knuYZbioUThzBhQSIOWvFbt24dUxjS2P JaTEBzucw73Lk0aW+g2oHxGRKHvRVZba1PbstY5lfntBXNHZnUILAuDZzdsLnRt3YY4t k5ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=EG0j8/P6IBog1adNhABbm2zVUlafoEOVKLf9OpJ4t9Y=; b=M3TEM7j5dY3GLN2Z1XKlulypoAR1uVj8CEQkWJqhRxdAQZYnn2Guwj8Z3LhiHH7974 cHBWAzJOlc7LAJc5FvucofTKaARChFeUGgUkr8Guh0DCCCnF6Armxj5BsF2LrvJTAmuy 7MLxDtk9n3nRI9D2NyRfCLdx4VU13wIrc1t8yicF2JGZxQlDIWK6K4I36p2y7/9DO0s9 ctCWKE8m5UIlmY/CVI6H/5aYedPHFqRltjR36wd5QfR3KwxUs14XzW11YDfK4tXlcLfP B5LKaUCJZ4az1/XpFCvk67jtBQ+KGn4fglsLpxQqgliGTdi21PIekc7VqOc/cKrCqN6v REaA== X-Gm-Message-State: AOAM531HchJOPbpoxYC72zfTDMo+1OfLcdAzM85kwyt4qEApzYC43ngK xiJ1GKrgw1fUG8wgfc1imK2bBA== X-Google-Smtp-Source: ABdhPJw7j/qDciNrIWkJYD93cuW24RVBlWqkheXgZGM2uZjhJ/o0HGRVDfrLfR3xzm5VAYmjEiX8uw== X-Received: by 2002:a92:1e12:: with SMTP id e18mr9056472ile.47.1616370946939; Sun, 21 Mar 2021 16:55:46 -0700 (PDT) Received: from pryzbyj.telsasoft (charmander.telsasoft.com. [50.244.222.1]) by smtp.gmail.com with ESMTPSA id g6sm6943421ilj.28.2021.03.21.16.55.46 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 21 Mar 2021 16:55:46 -0700 (PDT) Received: by pryzbyj.telsasoft (Postfix, from userid 1000) id 2C73B800870; Sun, 21 Mar 2021 18:55:45 -0500 (CDT) Date: Sun, 21 Mar 2021 18:55:45 -0500 From: Justin Pryzby To: Tom Lane , Robert Haas , Alvaro Herrera Cc: Dilip Kumar , Andres Freund , Tomas Vondra , Alexander Korotkov , David Steele , Ildus Kurbangaliev , Dmitry Dolgov <9erthalion6@gmail.com>, pgsql-hackers@lists.postgresql.org Subject: Re: [HACKERS] Custom compression methods Message-ID: <20210321235544.GD4203@telsasoft.com> References: <20210319222242.GA23930@alvherre.pgsql> <4158590.1616185209@sss.pgh.pa.us> <20210320000625.GP11765@telsasoft.com> <20210320074420.GR11765@telsasoft.com> <20210320232039.GY11765@telsasoft.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="GZVR6ND4mMseVXL/" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210320232039.GY11765@telsasoft.com> User-Agent: Mutt/1.9.4 (2018-02-28) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --GZVR6ND4mMseVXL/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, Mar 20, 2021 at 06:20:39PM -0500, Justin Pryzby wrote: > Rebased on HEAD. > 0005 forgot to update compression_1.out. > Included changes to ./configure.ac and some other patches, but not Tomas's, > since it'll make CFBOT get mad as soon as that's pushed. Rebased again. Renamed "t" to a badcompresstbl to avoid name conflicts. Polish the enum GUC patch some. I noticed that TOAST_INVALID_COMPRESSION_ID was unused ... but then I found a use for it. --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0001-Add-docs-for-default_toast_compression.patch" From edc0e6ab248600c6d33dd681359554550c88a679 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 19:12:53 -0500 Subject: [PATCH v3 01/12] Add docs for default_toast_compression.. bbe0a81db69bd10bd166907c3701492a29aca294 --- doc/src/sgml/config.sgml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index ee4925d6d9..5cb851a5eb 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8151,6 +8151,29 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; + + default_toast_compression (string) + + default_toast_compression configuration parameter + + + + + This parameter specifies the default compression method to use + when compressing data in TOAST tables. + It applies only to variable-width data types. + It may be overriden by compression clauses in the + CREATE command, or changed after the relation is + created by ALTER TABLE ... SET COMPRESSION. + + The supported compression methods are pglz and + (if configured at the time PostgreSQL was + built) lz4. + The default is pglz. + + + + temp_tablespaces (string) -- 2.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0002-doc-pg_dump-no-toast-compression.patch" From 02a9038381bdfe577aa32b67bcd231acd61f2c20 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 21:52:28 -0500 Subject: [PATCH v3 02/12] doc: pg_dump --no-toast-compression --- doc/src/sgml/ref/pg_dump.sgml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index bcbb7a25fb..989b8e2381 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -931,6 +931,18 @@ PostgreSQL documentation + + + + + Do not output commands to set TOAST compression + methods. + With this option, all objects will be created using whichever + compression method is the default during restore. + + + + -- 2.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0003-Compression-method-is-an-char-not-an-OID.patch" From 7e9b4b3c6f12abaf7531ef5109e084dc188e7dda Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 20:23:40 -0500 Subject: [PATCH v3 03/12] Compression method is an char not an OID --- src/backend/commands/tablecmds.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 877c08814e..22f3c5efc0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7847,6 +7847,7 @@ SetIndexStorageProperties(Relation rel, Relation attrelation, index_close(indrel, lockmode); } } + /* * ALTER TABLE ALTER COLUMN SET STORAGE * @@ -15070,7 +15071,7 @@ ATExecSetCompression(AlteredTableInfo *tab, AttrNumber attnum; char *compression; char typstorage; - Oid cmoid; + char cmethod; ObjectAddress address; Assert(IsA(newValue, String)); @@ -15104,10 +15105,10 @@ ATExecSetCompression(AlteredTableInfo *tab, format_type_be(atttableform->atttypid)))); /* get the attribute compression method. */ - cmoid = GetAttributeCompression(atttableform, compression); + cmethod = GetAttributeCompression(atttableform, compression); /* update pg_attribute entry */ - atttableform->attcompression = cmoid; + atttableform->attcompression = cmethod; CatalogTupleUpdate(attrel, &tuple->t_self, tuple); InvokeObjectPostAlterHook(RelationRelationId, @@ -15118,7 +15119,7 @@ ATExecSetCompression(AlteredTableInfo *tab, * Apply the change to indexes as well (only for simple index columns, * matching behavior of index.c ConstructTupleDescriptor()). */ - SetIndexStorageProperties(rel, attrel, attnum, cmoid, '\0', lockmode); + SetIndexStorageProperties(rel, attrel, attnum, cmethod, '\0', lockmode); heap_freetuple(tuple); -- 2.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0004-Remove-duplicative-macro.patch" From 2262c8ffb041e8a7452bda724c676b6118895f4b Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 20:07:31 -0500 Subject: [PATCH v3 04/12] Remove duplicative macro --- src/include/access/toast_compression.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h index 5c9220c171..44b73bd57c 100644 --- a/src/include/access/toast_compression.h +++ b/src/include/access/toast_compression.h @@ -50,8 +50,6 @@ typedef enum ToastCompressionId errdetail("This functionality requires the server to be built with lz4 support."), \ errhint("You need to rebuild PostgreSQL using --with-lz4."))) -#define IsValidCompression(cm) ((cm) != InvalidCompressionMethod) - #define IsStorageCompressible(storage) ((storage) != TYPSTORAGE_PLAIN && \ (storage) != TYPSTORAGE_EXTERNAL) -- 2.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0005-Error-on-invalid-compression-in-CREATE-and-ALTER.patch" From 2deb4e72773a0d0fe9cff73527b3464329b8763f Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 20:19:55 -0500 Subject: [PATCH v3 05/12] Error on invalid compression in CREATE and ALTER --- src/backend/commands/tablecmds.c | 10 +++++++--- src/test/regress/expected/compression.out | 6 ++++++ src/test/regress/expected/compression_1.out | 6 ++++++ src/test/regress/sql/compression.sql | 5 +++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 22f3c5efc0..54fea31e43 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -17863,9 +17863,13 @@ GetAttributeCompression(Form_pg_attribute att, char *compression) /* fallback to default compression if it's not specified */ if (compression == NULL) - cmethod = GetDefaultToastCompression(); - else - cmethod = CompressionNameToMethod(compression); + return GetDefaultToastCompression(); + + cmethod = CompressionNameToMethod(compression); + if (!CompressionMethodIsValid(cmethod)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("invalid compression method \"%s\"", compression))); return cmethod; } diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out index 18ac5f05bb..c2f2e0e230 100644 --- a/src/test/regress/expected/compression.out +++ b/src/test/regress/expected/compression.out @@ -347,4 +347,10 @@ SELECT length(f1) FROM cmmove3; 10040 (2 rows) +CREATE TABLE badcompresstbl (a text COMPRESSION I_Do_Not_Exist_Compression); -- fails +ERROR: invalid compression method "i_do_not_exist_compression" +CREATE TABLE badcompresstbl (a text); +ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -- fails +ERROR: invalid compression method "i_do_not_exist_compression" +DROP TABLE badcompresstbl; \set HIDE_TOAST_COMPRESSION true diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out index c4a2cea4cd..6626f8e927 100644 --- a/src/test/regress/expected/compression_1.out +++ b/src/test/regress/expected/compression_1.out @@ -340,4 +340,10 @@ SELECT length(f1) FROM cmmove3; 10000 (1 row) +CREATE TABLE badcompresstbl (a text COMPRESSION I_Do_Not_Exist_Compression); -- fails +ERROR: invalid compression method "i_do_not_exist_compression" +CREATE TABLE badcompresstbl (a text); +ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -- fails +ERROR: invalid compression method "i_do_not_exist_compression" +DROP TABLE badcompresstbl; \set HIDE_TOAST_COMPRESSION true diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql index e23669cc94..5e178be04a 100644 --- a/src/test/regress/sql/compression.sql +++ b/src/test/regress/sql/compression.sql @@ -137,4 +137,9 @@ SELECT length(f1) FROM cmmove1; SELECT length(f1) FROM cmmove2; SELECT length(f1) FROM cmmove3; +CREATE TABLE badcompresstbl (a text COMPRESSION I_Do_Not_Exist_Compression); -- fails +CREATE TABLE badcompresstbl (a text); +ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -- fails +DROP TABLE badcompresstbl; + \set HIDE_TOAST_COMPRESSION true -- 2.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0006-WIP-Change-default_toast_compression-GUC-to-an-en.patch" From 005ae4b7668a31dd4a7ab1c25ce3af3420346d2f Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 19 Mar 2021 19:39:10 -0500 Subject: [PATCH v3 06/12] WIP: Change default_toast_compression GUC to an enum.. ..since the final version of the TOAST lz4 patch doesn't require catalog access --- src/backend/access/common/toast_compression.c | 57 ++++--------------- src/backend/utils/misc/guc.c | 23 ++++---- src/include/access/toast_compression.h | 11 ++-- src/test/regress/expected/compression.out | 4 +- src/test/regress/expected/compression_1.out | 9 ++- 5 files changed, 33 insertions(+), 71 deletions(-) diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index 00af1740cf..aa28772159 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -23,8 +23,16 @@ #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_ID; + +const struct config_enum_entry default_toast_compression_options[] = { + {"pglz", TOAST_PGLZ_COMPRESSION_ID, false}, +#ifdef USE_LZ4 + {"lz4", TOAST_LZ4_COMPRESSION_ID, false}, +#endif + {NULL, 0, false} +}; /* * Compress a varlena using PGLZ. @@ -269,48 +277,3 @@ toast_get_compression_id(struct varlena *attr) return cmid; } - -/* - * Validate a new value for the default_toast_compression GUC. - */ -bool -check_default_toast_compression(char **newval, void **extra, GucSource source) -{ - if (**newval == '\0') - { - GUC_check_errdetail("%s cannot be empty.", - "default_toast_compression"); - return false; - } - - if (strlen(*newval) >= NAMEDATALEN) - { - GUC_check_errdetail("%s is too long (maximum %d characters).", - "default_toast_compression", NAMEDATALEN - 1); - return false; - } - - if (!CompressionMethodIsValid(CompressionNameToMethod(*newval))) - { - /* - * 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; - } - } - - return true; -} diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 4272cf4821..5480d3d088 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -518,6 +518,7 @@ extern const struct config_enum_entry archive_mode_options[]; extern const struct config_enum_entry recovery_target_action_options[]; extern const struct config_enum_entry sync_method_options[]; extern const struct config_enum_entry dynamic_shared_memory_options[]; +extern const struct config_enum_entry default_toast_compression_options[]; /* * GUC option variables that are exported from this module @@ -3953,17 +3954,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."), @@ -4605,6 +4595,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_ID, + 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..c5cea08ccd 100644 --- a/src/include/access/toast_compression.h +++ b/src/include/access/toast_compression.h @@ -16,10 +16,8 @@ #include "utils/guc.h" /* GUCs */ -extern char *default_toast_compression; - -/* default compression method if not specified. */ -#define DEFAULT_TOAST_COMPRESSION "pglz" +extern int default_toast_compression; +extern const struct config_enum_entry default_toast_compression_options[]; /* * Built-in compression method-id. The toast compression header will store @@ -78,7 +76,7 @@ GetCompressionMethodName(char method) * in the built-in methods then return InvalidCompressionMethod. */ static inline char -CompressionNameToMethod(char *compression) +CompressionNameToMethod(const char *compression) { if (strcmp(compression, "pglz") == 0) return TOAST_PGLZ_COMPRESSION; @@ -101,7 +99,8 @@ CompressionNameToMethod(char *compression) static inline char GetDefaultToastCompression(void) { - return CompressionNameToMethod(default_toast_compression); + Assert(default_toast_compression < TOAST_INVALID_COMPRESSION_ID); + return CompressionNameToMethod(default_toast_compression_options[default_toast_compression].name); } /* pglz compression/decompression routines */ 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.17.0 --GZVR6ND4mMseVXL/ Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="v3-0008-Commentary-about-slicing.patch" Content-Transfer-Encoding: 8bit From fc2b846ed482f5407278a171da3ce3c847425a00 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 20 Mar 2021 16:13:44 -0500 Subject: [PATCH v3 08/12] Commentary about slicing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My language, per gripe from Álvaro Herrera: | I updated the coverage script to use --with-lz4; results are updated. | While eyeballing the results I noticed this bit in | lz4_decompress_datum_slice(): | | + /* slice decompression not supported prior to 1.8.3 */ | + if (LZ4_versionNumber() < 10803) | + return lz4_decompress_datum(value); | | which I read as returning the complete decompressed datum if slice | decompression is not supported. I thought that was a bug, but looking | at the caller I realize that this isn't really a problem, since it's | detoast_attr_slice's responsibility to slice the result further -- no | bug, it's just wasteful. I suggest to add comments to this effect, | perhaps as the attached (feel free to reword, I think mine is awkward.) --- src/backend/access/common/detoast.c | 3 +++ src/backend/access/common/toast_compression.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c index bed50e8603..519fe8c6a6 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -498,6 +498,9 @@ toast_decompress_datum(struct varlena *attr) * Decompress the front of a compressed version of a varlena datum. * offset handling happens in detoast_attr_slice. * Here we just decompress a slice from the front. + * + * If slice decompression is not supported, the full datum is decompressed, and + * then sliced by detoast_attr_slice. */ static struct varlena * toast_decompress_datum_slice(struct varlena *attr, int32 slicelength) diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index aa28772159..9107d16445 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -213,6 +213,9 @@ lz4_decompress_datum(const struct varlena *value) /* * Decompress part of a varlena that was compressed using LZ4. + * + * If slice decompression is not supported, the full datum is decompressed, and + * then sliced by detoast_attr_slice. */ struct varlena * lz4_decompress_datum_slice(const struct varlena *value, int32 slicelength) -- 2.17.0 --GZVR6ND4mMseVXL/--