From b6885f3d1814d7aa777cd067e62ea55fd91e3b16 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 25 Jan 2024 15:50:16 +0530 Subject: [PATCH 1/2] Compression method support with inheritance The compression method specified while creating a child table overrides any parent compression methods, even if they conflict. If child doesn't specify a compression method and all its parents use the same compression method, the child inherits parents' compression method. A child must specify a compression method when its parents use conflicting compression methods. The compression property, even if empty, remains unchanged in a child inheriting from parent/s after its creation i.e. when using ALTER TABLE ... INHERIT. NOTE To the reviewer (may be included in the final commit message if necessary): Before this change the error detail would mention the first pair of conflicting parent compression method. But with this change it waits till the child specification is considered by which time we may have encountered many such conflicting pairs. Hence the error detail after this change does not report conflicting compression methods. Those can be obtained from parent definitions if necessary. The code to maintain list of all conflicting methods or even the first conflicting pair does not seem worth the convenience it offers. This change is inline with what we do with conflicting default values. Author: Ashutosh Bapat Discussion: https://www.postgresql.org/message-id/flat/24656cec-d6ef-4d15-8b5b-e8dfc9c833a7@eisentraut.org --- doc/src/sgml/ref/create_table.sgml | 8 ++ src/backend/commands/tablecmds.c | 67 +++++++----- src/test/regress/expected/compression.out | 101 ++++++++++++++++-- src/test/regress/expected/compression_1.out | 111 ++++++++++++++++++-- src/test/regress/sql/compression.sql | 43 +++++++- 5 files changed, 289 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 4cbaaccaf7..3e5e4b107e 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -401,6 +401,14 @@ WITH ( MODULUS numeric_literal, REM Column STORAGE settings are also copied from parent tables. + + Explicitly specified COMPRESSION settings of a column + override any inherited COMPRESSION settings of that + column. Otheriwse any parents that specify COMPRESSION + settings for the column must all specify the same settings, which will be + inherited by the column in the new table, or an error will be reported. + + If a column in the parent table is an identity column, that property is not inherited. A column in the child table can be declared identity diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9f51696740..dc1e97d3f4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -350,6 +350,14 @@ typedef struct ForeignTruncateInfo #define child_dependency_type(child_is_partition) \ ((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL) +/* + * Bogus compression method name to track conflict in inherited compression + * property of attributes. It can be any string which does not look like a valid + * compression method. It is meant to be used by MergeAttributes() and its + * minions. It is not expected to be stored on disk. + */ +#define BOGUS_COMPRESSION_METHOD "*** conflicting compression ***" + static void truncate_check_rel(Oid relid, Form_pg_class reltuple); static void truncate_check_perms(Oid relid, Form_pg_class reltuple); static void truncate_check_activity(Relation rel); @@ -360,7 +368,8 @@ static List *MergeAttributes(List *columns, const List *supers, char relpersiste List **supnotnulls); static List *MergeCheckConstraint(List *constraints, const char *name, Node *expr); static void MergeChildAttribute(List *inh_columns, int exist_attno, int newcol_attno, const ColumnDef *newdef); -static ColumnDef *MergeInheritedAttribute(List *inh_columns, int exist_attno, const ColumnDef *newdef); +static ColumnDef *MergeInheritedAttribute(List *inh_columns, int exist_attno, const ColumnDef *newdef, + bool *have_deferred_conflicts); static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispartition); static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel); static void StoreCatalogInheritance(Oid relationId, List *supers, @@ -2483,7 +2492,7 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, List *inh_columns = NIL; List *constraints = NIL; List *nnconstraints = NIL; - bool have_bogus_defaults = false; + bool have_deferred_conflicts = false; int child_attno; static Node bogus_marker = {0}; /* marks conflicting defaults */ List *saved_columns = NIL; @@ -2744,7 +2753,8 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, /* * Yes, try to merge the two column definitions. */ - mergeddef = MergeInheritedAttribute(inh_columns, exist_attno, newdef); + mergeddef = MergeInheritedAttribute(inh_columns, exist_attno, newdef, + &have_deferred_conflicts); newattmap->attnums[parent_attno - 1] = exist_attno; @@ -2867,7 +2877,7 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, else if (!equal(def->cooked_default, this_default)) { def->cooked_default = &bogus_marker; - have_bogus_defaults = true; + have_deferred_conflicts = true; } } @@ -3077,10 +3087,11 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, } /* - * If we found any conflicting parent default values, check to make sure - * they were overridden by the child. + * If we found any conflicting parent default values or conflicting parent + * compression properties, check to make sure they were overridden by the + * child. */ - if (have_bogus_defaults) + if (have_deferred_conflicts) { foreach(lc, columns) { @@ -3101,6 +3112,14 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, def->colname), errhint("To resolve the conflict, specify a default explicitly."))); } + + if (def->compression != NULL && + strcmp(def->compression, BOGUS_COMPRESSION_METHOD) == 0) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("column \"%s\" inherits conflicting compression methods", + def->colname), + errhint("To resolve the conflict, specify a compression method explicitly."))); } } @@ -3264,19 +3283,11 @@ MergeChildAttribute(List *inh_columns, int exist_attno, int newcol_attno, const storage_name(newdef->storage)))); /* - * Copy compression parameter + * Child compression specification, if any, overrides inherited + * compression property. */ - if (inhdef->compression == NULL) + if (newdef->compression != NULL) inhdef->compression = newdef->compression; - else if (newdef->compression != NULL) - { - if (strcmp(inhdef->compression, newdef->compression) != 0) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("column \"%s\" has a compression method conflict", - attributeName), - errdetail("%s versus %s", inhdef->compression, newdef->compression))); - } /* * Merge of not-null constraints = OR 'em together @@ -3343,6 +3354,10 @@ MergeChildAttribute(List *inh_columns, int exist_attno, int newcol_attno, const * 'exist_attno' is the number the existing matching attribute in inh_columns. * 'newdef' is the new parent column/attribute definition to be merged. * + * Output arguments: + * 'have_deferred_conflicts' is set to true if there is a conflict in inherited + * compression properties; remains unchanged otherwise. + * * The matching ColumnDef in 'inh_columns' list is modified and returned. * * Notes: @@ -3356,7 +3371,8 @@ MergeChildAttribute(List *inh_columns, int exist_attno, int newcol_attno, const static ColumnDef * MergeInheritedAttribute(List *inh_columns, int exist_attno, - const ColumnDef *newdef) + const ColumnDef *newdef, + bool *have_deferred_conflicts) { char *attributeName = newdef->colname; ColumnDef *prevdef; @@ -3419,12 +3435,13 @@ MergeInheritedAttribute(List *inh_columns, */ if (prevdef->compression == NULL) prevdef->compression = newdef->compression; - else if (strcmp(prevdef->compression, newdef->compression) != 0) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("column \"%s\" has a compression method conflict", - attributeName), - errdetail("%s versus %s", prevdef->compression, newdef->compression))); + else if (newdef->compression != NULL && + strcmp(prevdef->compression, newdef->compression) != 0) + { + /* Note the conflict. */ + prevdef->compression = BOGUS_COMPRESSION_METHOD; + *have_deferred_conflicts = true; + } /* * Check for GENERATED conflicts diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out index 834b7555cb..c22a71f8bd 100644 --- a/src/test/regress/expected/compression.out +++ b/src/test/regress/expected/compression.out @@ -223,15 +223,102 @@ SELECT pg_column_compression(f1) FROM cmpart2; pglz (1 row) --- test compression with inheritance, error -CREATE TABLE cminh() INHERITS(cmdata, cmdata1); +-- test compression with inheritance +CREATE TABLE cmparent1 (f1 TEXT COMPRESSION pglz); +INSERT INTO cmparent1 VALUES ('cmparent1_' || repeat('1234567890', 1000)); +CREATE TABLE cmparent2 (f1 TEXT COMPRESSION lz4); +INSERT INTO cmparent2 VALUES ('cmparent2_' || repeat('1234567890', 1000)); +CREATE TABLE ncmparent (f1 TEXT); -- parent without compression +INSERT INTO ncmparent VALUES ('ncmparent_' || repeat('1234567890', 1000)); +CREATE TABLE cminh1(f1 TEXT) INHERITS(cmparent1); +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh1 VALUES ('cminh1_' || repeat('1234567890', 1000)); +CREATE TABLE cminh2(f1 TEXT) INHERITS(ncmparent, cmparent1); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh2 VALUES ('cminh2_' || repeat('1234567890', 1000)); +CREATE TABLE cminh3(f1 TEXT) INHERITS(cmparent1, ncmparent); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh3 VALUES ('cminh3_' || repeat('1234567890', 1000)); +-- conflicting compression methods from parents +CREATE TABLE cminh() INHERITS(cmparent1, cmparent2); --error +NOTICE: merging multiple inherited definitions of column "f1" +ERROR: column "f1" inherits conflicting compression methods +HINT: To resolve the conflict, specify a compression method explicitly. +CREATE TABLE cminh(f1 TEXT) INHERITS(cmparent1, cmparent2); --error NOTICE: merging multiple inherited definitions of column "f1" -ERROR: column "f1" has a compression method conflict -DETAIL: pglz versus lz4 -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 +ERROR: column "f1" inherits conflicting compression methods +HINT: To resolve the conflict, specify a compression method explicitly. +-- child compression specification takes precedence, even if parent's +-- compression conflict +CREATE TABLE cminh4(f1 TEXT COMPRESSION lz4) INHERITS(cmparent1); +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh4 VALUES ('cminh4_' || repeat('1234567890', 1000)); +CREATE TABLE cminh5(f1 TEXT COMPRESSION pglz) INHERITS(cmparent1, cmparent2); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh5 VALUES ('cminh5_' || repeat('1234567890', 1000)); +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent1; + tableoid | pg_column_compression +-----------+----------------------- + cmparent1 | pglz + cminh1 | pglz + cminh2 | pglz + cminh3 | pglz + cminh4 | lz4 + cminh5 | pglz +(6 rows) + +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent2; + tableoid | pg_column_compression +-----------+----------------------- + cmparent2 | lz4 + cminh5 | pglz +(2 rows) + +SELECT tableoid::regclass, pg_column_compression(f1) FROM ncmparent; + tableoid | pg_column_compression +-----------+----------------------- + ncmparent | pglz + cminh2 | pglz + cminh3 | pglz +(3 rows) + +-- ALTER compression specification in child +ALTER TABLE cminh1 ALTER COLUMN f1 SET COMPRESSION lz4; +INSERT INTO cminh1 VALUES ('cminh1_lz4_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh1; + pg_column_compression +----------------------- + pglz + lz4 +(2 rows) + +-- INHERIT through ALTER TABLE +CREATE TABLE cminh6 (f1 TEXT); +INSERT INTO cminh6 VALUES ('cminh6_' || repeat('1234567890', 1000)); +ALTER TABLE cminh6 INHERIT cmparent1; +INSERT INTO cminh6 VALUES ('cminh6_inh_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh6; + pg_column_compression +----------------------- + pglz + pglz +(2 rows) + +CREATE TABLE cminh7 (f1 TEXT COMPRESSION lz4); +INSERT INTO cminh7 VALUES ('cminh7_' || repeat('1234567890', 1000)); +ALTER TABLE cminh7 INHERIT cmparent1; +INSERT INTO cminh7 VALUES ('cminh7_inh_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh7; + pg_column_compression +----------------------- + lz4 + lz4 +(2 rows) + -- test default_toast_compression GUC SET default_toast_compression = ''; ERROR: invalid value for parameter "default_toast_compression": "" diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out index ddcd137c49..d70155d5d0 100644 --- a/src/test/regress/expected/compression_1.out +++ b/src/test/regress/expected/compression_1.out @@ -216,13 +216,112 @@ SELECT pg_column_compression(f1) FROM cmpart2; ----------------------- (0 rows) --- test compression with inheritance, error -CREATE TABLE cminh() INHERITS(cmdata, cmdata1); -ERROR: relation "cmdata1" does not exist -CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); +-- test compression with inheritance +CREATE TABLE cmparent1 (f1 TEXT COMPRESSION pglz); +INSERT INTO cmparent1 VALUES ('cmparent1_' || repeat('1234567890', 1000)); +CREATE TABLE cmparent2 (f1 TEXT COMPRESSION lz4); +ERROR: compression method lz4 not supported +DETAIL: This functionality requires the server to be built with lz4 support. +INSERT INTO cmparent2 VALUES ('cmparent2_' || repeat('1234567890', 1000)); +ERROR: relation "cmparent2" does not exist +LINE 1: INSERT INTO cmparent2 VALUES ('cmparent2_' || repeat('123456... + ^ +CREATE TABLE ncmparent (f1 TEXT); -- parent without compression +INSERT INTO ncmparent VALUES ('ncmparent_' || repeat('1234567890', 1000)); +CREATE TABLE cminh1(f1 TEXT) INHERITS(cmparent1); NOTICE: merging column "f1" with inherited definition -ERROR: column "f1" has a compression method conflict -DETAIL: pglz versus lz4 +INSERT INTO cminh1 VALUES ('cminh1_' || repeat('1234567890', 1000)); +CREATE TABLE cminh2(f1 TEXT) INHERITS(ncmparent, cmparent1); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh2 VALUES ('cminh2_' || repeat('1234567890', 1000)); +CREATE TABLE cminh3(f1 TEXT) INHERITS(cmparent1, ncmparent); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging column "f1" with inherited definition +INSERT INTO cminh3 VALUES ('cminh3_' || repeat('1234567890', 1000)); +-- conflicting compression methods from parents +CREATE TABLE cminh() INHERITS(cmparent1, cmparent2); --error +ERROR: relation "cmparent2" does not exist +CREATE TABLE cminh(f1 TEXT) INHERITS(cmparent1, cmparent2); --error +ERROR: relation "cmparent2" does not exist +-- child compression specification takes precedence, even if parent's +-- compression conflict +CREATE TABLE cminh4(f1 TEXT COMPRESSION lz4) INHERITS(cmparent1); +NOTICE: merging column "f1" with inherited definition +ERROR: compression method lz4 not supported +DETAIL: This functionality requires the server to be built with lz4 support. +INSERT INTO cminh4 VALUES ('cminh4_' || repeat('1234567890', 1000)); +ERROR: relation "cminh4" does not exist +LINE 1: INSERT INTO cminh4 VALUES ('cminh4_' || repeat('1234567890',... + ^ +CREATE TABLE cminh5(f1 TEXT COMPRESSION pglz) INHERITS(cmparent1, cmparent2); +ERROR: relation "cmparent2" does not exist +INSERT INTO cminh5 VALUES ('cminh5_' || repeat('1234567890', 1000)); +ERROR: relation "cminh5" does not exist +LINE 1: INSERT INTO cminh5 VALUES ('cminh5_' || repeat('1234567890',... + ^ +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent1; + tableoid | pg_column_compression +-----------+----------------------- + cmparent1 | pglz + cminh1 | pglz + cminh2 | pglz + cminh3 | pglz +(4 rows) + +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent2; +ERROR: relation "cmparent2" does not exist +LINE 1: ...ableoid::regclass, pg_column_compression(f1) FROM cmparent2; + ^ +SELECT tableoid::regclass, pg_column_compression(f1) FROM ncmparent; + tableoid | pg_column_compression +-----------+----------------------- + ncmparent | pglz + cminh2 | pglz + cminh3 | pglz +(3 rows) + +-- ALTER compression specification in child +ALTER TABLE cminh1 ALTER COLUMN f1 SET COMPRESSION lz4; +ERROR: compression method lz4 not supported +DETAIL: This functionality requires the server to be built with lz4 support. +INSERT INTO cminh1 VALUES ('cminh1_lz4_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh1; + pg_column_compression +----------------------- + pglz + pglz +(2 rows) + +-- INHERIT through ALTER TABLE +CREATE TABLE cminh6 (f1 TEXT); +INSERT INTO cminh6 VALUES ('cminh6_' || repeat('1234567890', 1000)); +ALTER TABLE cminh6 INHERIT cmparent1; +INSERT INTO cminh6 VALUES ('cminh6_inh_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh6; + pg_column_compression +----------------------- + pglz + pglz +(2 rows) + +CREATE TABLE cminh7 (f1 TEXT COMPRESSION lz4); +ERROR: compression method lz4 not supported +DETAIL: This functionality requires the server to be built with lz4 support. +INSERT INTO cminh7 VALUES ('cminh7_' || repeat('1234567890', 1000)); +ERROR: relation "cminh7" does not exist +LINE 1: INSERT INTO cminh7 VALUES ('cminh7_' || repeat('1234567890',... + ^ +ALTER TABLE cminh7 INHERIT cmparent1; +ERROR: relation "cminh7" does not exist +INSERT INTO cminh7 VALUES ('cminh7_inh_' || repeat('1234567890', 1000)); +ERROR: relation "cminh7" does not exist +LINE 1: INSERT INTO cminh7 VALUES ('cminh7_inh_' || repeat('12345678... + ^ +SELECT pg_column_compression(f1) FROM cminh7; +ERROR: relation "cminh7" does not exist +LINE 1: SELECT pg_column_compression(f1) FROM cminh7; + ^ -- test default_toast_compression GUC SET default_toast_compression = ''; ERROR: invalid value for parameter "default_toast_compression": "" diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql index 7179a5002e..ad8e7afd2a 100644 --- a/src/test/regress/sql/compression.sql +++ b/src/test/regress/sql/compression.sql @@ -93,9 +93,46 @@ INSERT INTO cmpart VALUES (repeat('123456789', 4004)); SELECT pg_column_compression(f1) FROM cmpart1; SELECT pg_column_compression(f1) FROM cmpart2; --- test compression with inheritance, error -CREATE TABLE cminh() INHERITS(cmdata, cmdata1); -CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); +-- test compression with inheritance +CREATE TABLE cmparent1 (f1 TEXT COMPRESSION pglz); +INSERT INTO cmparent1 VALUES ('cmparent1_' || repeat('1234567890', 1000)); +CREATE TABLE cmparent2 (f1 TEXT COMPRESSION lz4); +INSERT INTO cmparent2 VALUES ('cmparent2_' || repeat('1234567890', 1000)); +CREATE TABLE ncmparent (f1 TEXT); -- parent without compression +INSERT INTO ncmparent VALUES ('ncmparent_' || repeat('1234567890', 1000)); +CREATE TABLE cminh1(f1 TEXT) INHERITS(cmparent1); +INSERT INTO cminh1 VALUES ('cminh1_' || repeat('1234567890', 1000)); +CREATE TABLE cminh2(f1 TEXT) INHERITS(ncmparent, cmparent1); +INSERT INTO cminh2 VALUES ('cminh2_' || repeat('1234567890', 1000)); +CREATE TABLE cminh3(f1 TEXT) INHERITS(cmparent1, ncmparent); +INSERT INTO cminh3 VALUES ('cminh3_' || repeat('1234567890', 1000)); +-- conflicting compression methods from parents +CREATE TABLE cminh() INHERITS(cmparent1, cmparent2); --error +CREATE TABLE cminh(f1 TEXT) INHERITS(cmparent1, cmparent2); --error +-- child compression specification takes precedence, even if parent's +-- compression conflict +CREATE TABLE cminh4(f1 TEXT COMPRESSION lz4) INHERITS(cmparent1); +INSERT INTO cminh4 VALUES ('cminh4_' || repeat('1234567890', 1000)); +CREATE TABLE cminh5(f1 TEXT COMPRESSION pglz) INHERITS(cmparent1, cmparent2); +INSERT INTO cminh5 VALUES ('cminh5_' || repeat('1234567890', 1000)); +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent1; +SELECT tableoid::regclass, pg_column_compression(f1) FROM cmparent2; +SELECT tableoid::regclass, pg_column_compression(f1) FROM ncmparent; +-- ALTER compression specification in child +ALTER TABLE cminh1 ALTER COLUMN f1 SET COMPRESSION lz4; +INSERT INTO cminh1 VALUES ('cminh1_lz4_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh1; +-- INHERIT through ALTER TABLE +CREATE TABLE cminh6 (f1 TEXT); +INSERT INTO cminh6 VALUES ('cminh6_' || repeat('1234567890', 1000)); +ALTER TABLE cminh6 INHERIT cmparent1; +INSERT INTO cminh6 VALUES ('cminh6_inh_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh6; +CREATE TABLE cminh7 (f1 TEXT COMPRESSION lz4); +INSERT INTO cminh7 VALUES ('cminh7_' || repeat('1234567890', 1000)); +ALTER TABLE cminh7 INHERIT cmparent1; +INSERT INTO cminh7 VALUES ('cminh7_inh_' || repeat('1234567890', 1000)); +SELECT pg_column_compression(f1) FROM cminh7; -- test default_toast_compression GUC SET default_toast_compression = ''; -- 2.25.1