On Mon, Apr 29, 2024 at 6:46 PM Robert Haas <robertmh...@gmail.com> wrote:
> On Sat, Apr 20, 2024 at 12:17 AM Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: > > Yes please. Probably this issue surfaced again after we reverted > compression and storage fix? Please If that's the case, please add it to > the open items. > > This is still on the open items list and I'm not clear who, if anyone, > is working on fixing it. > > It would be good if someone fixed it. :-) > Here's a patch fixing it. I have added the reproducer provided by Alexander as a test. I thought of improving that test further to test the compression of the inherited table but did not implement it since we haven't documented the behaviour of compression with inheritance. Defining and implementing compression behaviour for inherited tables was the goal of 0413a556990ba628a3de8a0b58be020fd9a14ed0, which has been reverted. -- Best Wishes, Ashutosh Bapat
From 7c1ff7b17933eef9523486a2c0a054836db9cf24 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Tue, 30 Apr 2024 11:19:43 +0530 Subject: [PATCH] Fix segmentation fault in MergeInheritedAttribute() While converting a pg_attribute tuple into a ColumnDef, ColumnDef::compression remains NULL if there is no compression method set fot the attribute. Calling strcmp() with NULL ColumnDef::compression, when comparing compression methods of parents, causes segmentation fault in MergeInheritedAttribute(). Skip comparing compression methods if either of them is NULL. Author: Ashutosh Bapat Discussion: https://www.postgresql.org/message-id/b22a6834-aacb-7b18-0424-a3f5fe889667%40gmail.com --- src/backend/commands/tablecmds.c | 16 ++++++++++------ src/test/regress/expected/compression.out | 10 +++++++--- src/test/regress/sql/compression.sql | 8 +++++--- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3556240c8e..e29f96e357 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3430,12 +3430,16 @@ 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) + { + 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))); + } /* * Check for GENERATED conflicts diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out index 834b7555cb..4dd9ee7200 100644 --- a/src/test/regress/expected/compression.out +++ b/src/test/regress/expected/compression.out @@ -223,15 +223,18 @@ 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 cminh() INHERITS(cmdata, cmdata1); -- 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); +CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); -- error NOTICE: merging column "f1" with inherited definition ERROR: column "f1" has a compression method conflict DETAIL: pglz versus lz4 +CREATE TABLE cmdata3(f1 text); +CREATE TABLE cminh() INHERITS (cmdata, cmdata3); +NOTICE: merging multiple inherited definitions of column "f1" -- test default_toast_compression GUC SET default_toast_compression = ''; ERROR: invalid value for parameter "default_toast_compression": "" @@ -251,6 +254,7 @@ INSERT INTO cmdata VALUES (repeat('123456789', 4004)); f1 | text | | | | extended | lz4 | | Indexes: "idx" btree (f1) +Child tables: cminh SELECT pg_column_compression(f1) FROM cmdata; pg_column_compression diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql index 7179a5002e..490595fcfb 100644 --- a/src/test/regress/sql/compression.sql +++ b/src/test/regress/sql/compression.sql @@ -93,9 +93,11 @@ 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 cminh() INHERITS(cmdata, cmdata1); -- error +CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); -- error +CREATE TABLE cmdata3(f1 text); +CREATE TABLE cminh() INHERITS (cmdata, cmdata3); -- test default_toast_compression GUC SET default_toast_compression = ''; -- 2.34.1
