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.
>From edc0e6ab248600c6d33dd681359554550c88a679 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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;
</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 parameter specifies the default compression method to use
+ when compressing data in <acronym>TOAST</acronym> tables.
+ It applies only to variable-width data types.
+ It may be overriden by compression clauses in the
+ <command>CREATE</command> command, or changed after the relation is
+ created by <command>ALTER TABLE ... SET COMPRESSION</command>.
+
+ 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>
--
2.17.0
>From 02a9038381bdfe577aa32b67bcd231acd61f2c20 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--no-toast-compression</option></term>
+ <listitem>
+ <para>
+ Do not output commands to set <acronym>TOAST</acronym> compression
+ methods.
+ With this option, all objects will be created using whichever
+ compression method is the default during restore.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--no-unlogged-table-data</option></term>
<listitem>
--
2.17.0
>From 7e9b4b3c6f12abaf7531ef5109e084dc188e7dda Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
>From 2262c8ffb041e8a7452bda724c676b6118895f4b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
>From 2deb4e72773a0d0fe9cff73527b3464329b8763f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
>From 005ae4b7668a31dd4a7ab1c25ce3af3420346d2f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
>From fc2b846ed482f5407278a171da3ce3c847425a00 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
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
