public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 6/8] Add default_toast_compression GUC 3+ messages / 3 participants [nested] [flat]
* [PATCH 6/8] Add default_toast_compression GUC @ 2021-03-10 08:35 Dilip Kumar <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Dilip Kumar @ 2021-03-10 08:35 UTC (permalink / raw) Justin Pryzby and Dilip Kumar --- src/backend/access/common/toast_compression.c | 46 +++++++++++++++++++ src/backend/access/common/tupdesc.c | 2 +- src/backend/bootstrap/bootstrap.c | 2 +- src/backend/commands/tablecmds.c | 8 ++-- src/backend/utils/misc/guc.c | 12 +++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/access/toast_compression.h | 22 ++++++++- src/test/regress/expected/compression.out | 16 +++++++ src/test/regress/expected/compression_1.out | 19 ++++++++ src/test/regress/sql/compression.sql | 8 ++++ 10 files changed, 128 insertions(+), 8 deletions(-) diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index db4911ce43..27fe47b201 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -55,6 +55,9 @@ const CompressionRoutine toast_compression[] = } }; +/* Compile-time default */ +char *default_toast_compression = DEFAULT_TOAST_COMPRESSION; + /* * pglz_cmcompress - compression routine for pglz compression method * @@ -306,3 +309,46 @@ GetCompressionRoutines(char method) { return &toast_compression[CompressionMethodToId(method)]; } + +/* check_hook: validate new default_toast_compression */ +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/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index a66d1113db..362a371a6c 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -668,7 +668,7 @@ TupleDescInitEntry(TupleDesc desc, att->attcollation = typeForm->typcollation; if (IsStorageCompressible(typeForm->typstorage)) - att->attcompression = DefaultCompressionMethod; + att->attcompression = GetDefaultToastCompression(); else att->attcompression = InvalidCompressionMethod; diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index af0c0aa77e..9be0841d08 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -733,7 +733,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness) attrtypes[attnum]->atttypmod = -1; attrtypes[attnum]->attislocal = true; if (IsStorageCompressible(attrtypes[attnum]->attstorage)) - attrtypes[attnum]->attcompression = DefaultCompressionMethod; + attrtypes[attnum]->attcompression = GetDefaultToastCompression(); else attrtypes[attnum]->attcompression = InvalidCompressionMethod; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e20dd5f63c..a0d2af0cde 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -11971,7 +11971,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, if (!IsStorageCompressible(tform->typstorage)) attTup->attcompression = InvalidCompressionMethod; else if (!CompressionMethodIsValid(attTup->attcompression)) - attTup->attcompression = DefaultCompressionMethod; + attTup->attcompression = GetDefaultToastCompression(); } else attTup->attcompression = InvalidCompressionMethod; @@ -17847,9 +17847,9 @@ GetAttributeCompression(Form_pg_attribute att, char *compression) /* fallback to default compression if it's not specified */ if (compression == NULL) - return DefaultCompressionMethod; - - cmethod = CompressionNameToMethod(compression); + cmethod = GetDefaultToastCompression(); + else + cmethod = CompressionNameToMethod(compression); return cmethod; } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 953837085f..e59ed5b214 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -33,6 +33,7 @@ #include "access/gin.h" #include "access/rmgr.h" #include "access/tableam.h" +#include "access/toast_compression.h" #include "access/transam.h" #include "access/twophase.h" #include "access/xact.h" @@ -3915,6 +3916,17 @@ 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."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index f46c2dd7a8..7d7a433dcc 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -659,6 +659,7 @@ #temp_tablespaces = '' # a list of tablespace names, '' uses # only default tablespace #default_table_access_method = 'heap' +#default_toast_compression = 'pglz' # 'pglz' or 'lz4' #check_function_bodies = on #default_transaction_isolation = 'read committed' #default_transaction_read_only = off diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h index 38800cb97a..e9d9ce5634 100644 --- a/src/include/access/toast_compression.h +++ b/src/include/access/toast_compression.h @@ -15,6 +15,14 @@ #include "postgres.h" +#include "utils/guc.h" + +/* default compression method if not specified. */ +#define DEFAULT_TOAST_COMPRESSION "pglz" + +/* GUCs */ +extern char *default_toast_compression; + /* * Built-in compression methods. pg_attribute will store this in the * attcompression column. @@ -36,8 +44,6 @@ typedef enum CompressionId LZ4_COMPRESSION_ID = 1 } CompressionId; -/* use default compression method if it is not specified. */ -#define DefaultCompressionMethod PGLZ_COMPRESSION #define IsValidCompression(cm) ((cm) != InvalidCompressionMethod) #define IsStorageCompressible(storage) ((storage) != TYPSTORAGE_PLAIN && \ @@ -67,6 +73,8 @@ typedef struct CompressionRoutine extern char CompressionNameToMethod(char *compression); extern const CompressionRoutine *GetCompressionRoutines(char method); +extern bool check_default_toast_compression(char **newval, void **extra, + GucSource source); /* * CompressionMethodToId - Convert compression method to compression id. @@ -115,5 +123,15 @@ GetCompressionMethodName(char method) return GetCompressionRoutines(method)->cmname; } +/* + * GetDefaultToastCompression -- get the current toast compression + * + * This exists to hide the use of the default_toast_compression GUC variable. + */ +static inline char +GetDefaultToastCompression(void) +{ + return CompressionNameToMethod(default_toast_compression); +} #endif /* TOAST_COMPRESSION_H */ diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out index 6981d580dc..9ddffc8b18 100644 --- a/src/test/regress/expected/compression.out +++ b/src/test/regress/expected/compression.out @@ -231,6 +231,22 @@ CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); NOTICE: merging column "f1" with inherited definition ERROR: column "f1" has a compression method conflict 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. +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. +SET default_toast_compression = 'lz4'; +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + Table "public.cmdata2" + Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description +--------+------+-----------+----------+---------+----------+-------------+--------------+------------- + f1 | text | | | | extended | lz4 | | + -- check data is ok SELECT length(f1) FROM cmdata; length diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out index 893c18c7fa..03a7f46554 100644 --- a/src/test/regress/expected/compression_1.out +++ b/src/test/regress/expected/compression_1.out @@ -237,6 +237,25 @@ CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); NOTICE: merging column "f1" with inherited definition ERROR: column "f1" has a compression method conflict 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. +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. +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. +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + Table "public.cmdata2" + Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description +--------+------+-----------+----------+---------+----------+-------------+--------------+------------- + f1 | text | | | | extended | pglz | | + -- check data is ok SELECT length(f1) FROM cmdata; length diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql index b085c9fbd9..6c9b24f1f7 100644 --- a/src/test/regress/sql/compression.sql +++ b/src/test/regress/sql/compression.sql @@ -103,6 +103,14 @@ SELECT pg_column_compression(f1) FROM cmpart; CREATE TABLE cminh() INHERITS(cmdata, cmdata1); CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); +-- test default_toast_compression GUC +SET default_toast_compression = ''; +SET default_toast_compression = 'I do not exist compression'; +SET default_toast_compression = 'lz4'; +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + -- check data is ok SELECT length(f1) FROM cmdata; SELECT length(f1) FROM cmdata1; -- 2.17.0 --C94crkcyjafcjHxo Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0007-Alter-table-set-compression.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] json_lex_string: don't overread on bad UTF8 @ 2024-05-07 03:42 Michael Paquier <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Michael Paquier @ 2024-05-07 03:42 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers; Andrew Dunstan <[email protected]> On Fri, May 03, 2024 at 07:05:38AM -0700, Jacob Champion wrote: > On Fri, May 3, 2024 at 4:54 AM Peter Eisentraut <[email protected]> wrote: >> but for the general encoding conversion we have what >> would appear to be the same behavior in report_invalid_encoding(), and >> we go out of our way there to produce a verbose error message including >> the invalid data. I was looking for that a couple of days ago in the backend but could not put my finger on it. Thanks. > We could port something like that to src/common. IMO that'd be more > suited for an actual conversion routine, though, as opposed to a > parser that for the most part assumes you didn't lie about the input > encoding and is just trying not to crash if you're wrong. Most of the > time, the parser just copies bytes between delimiters around and it's > up to the caller to handle encodings... the exceptions to that are the > \uXXXX escapes and the error handling. Hmm. That would still leave the backpatch issue at hand, which is kind of confusing to leave as it is. Would it be complicated to truncate the entire byte sequence in the error message and just give up because we cannot do better if the input byte sequence is incomplete? We could still have some information depending on the string given in input, which should be enough, hopefully. With the location pointing to the beginning of the sequence, even better. > Offhand, are all of our supported frontend encodings > self-synchronizing? By that I mean, is it safe to print a partial byte > sequence if the locale isn't UTF-8? (As I type this I'm starting at > Shift-JIS, and thinking "probably not.") > > Actually -- hopefully this is not too much of a tangent -- that > further crystallizes a vague unease about the API that I have. The > JsonLexContext is initialized with something called the > "input_encoding", but that encoding is necessarily also the output > encoding for parsed string literals and error messages. For the server > side that's fine, but frontend clients have the input_encoding locked > to UTF-8, which seems like it might cause problems? Maybe I'm missing > code somewhere, but I don't see a conversion routine from > json_errdetail() to the actual client/locale encoding. (And the parser > does not support multibyte input_encodings that contain ASCII in trail > bytes.) Referring to json_lex_string() that does UTF-8 -> ASCII -> give-up in its conversion for FRONTEND, I guess? Yep. This limitation looks like a problem, especially if plugging that to libpq. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] json_lex_string: don't overread on bad UTF8 @ 2024-05-07 21:06 Jacob Champion <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Jacob Champion @ 2024-05-07 21:06 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers; Andrew Dunstan <[email protected]> On Mon, May 6, 2024 at 8:43 PM Michael Paquier <[email protected]> wrote: > On Fri, May 03, 2024 at 07:05:38AM -0700, Jacob Champion wrote: > > We could port something like that to src/common. IMO that'd be more > > suited for an actual conversion routine, though, as opposed to a > > parser that for the most part assumes you didn't lie about the input > > encoding and is just trying not to crash if you're wrong. Most of the > > time, the parser just copies bytes between delimiters around and it's > > up to the caller to handle encodings... the exceptions to that are the > > \uXXXX escapes and the error handling. > > Hmm. That would still leave the backpatch issue at hand, which is > kind of confusing to leave as it is. Would it be complicated to > truncate the entire byte sequence in the error message and just give > up because we cannot do better if the input byte sequence is > incomplete? Maybe I've misunderstood, but isn't that what's being done in v2? > > Maybe I'm missing > > code somewhere, but I don't see a conversion routine from > > json_errdetail() to the actual client/locale encoding. (And the parser > > does not support multibyte input_encodings that contain ASCII in trail > > bytes.) > > Referring to json_lex_string() that does UTF-8 -> ASCII -> give-up in > its conversion for FRONTEND, I guess? Yep. This limitation looks > like a problem, especially if plugging that to libpq. Okay. How we deal with that will likely guide the "optimal" fix to error reporting, I think... Thanks, --Jacob ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-05-07 21:06 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-10 08:35 [PATCH 6/8] Add default_toast_compression GUC Dilip Kumar <[email protected]> 2024-05-07 03:42 Re: [PATCH] json_lex_string: don't overread on bad UTF8 Michael Paquier <[email protected]> 2024-05-07 21:06 ` Re: [PATCH] json_lex_string: don't overread on bad UTF8 Jacob Champion <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox