On Wed, Mar 24, 2021 at 10:57 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Wed, Mar 24, 2021 at 12:14 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
> > Actually, we are already doing this, I mean ALTER TABLE .. ALTER
> > COLUMN .. SET COMPRESSION is already updating the compression method
> > of the index attribute. So 0003 doesn't make sense, sorry for the
> > noise. However, 0001 and 0002 are still valid, or do you think that
> > we don't want 0001 also? If we don't need 0001 also then we need to
> > update the test output for 0002 slightly.
>
> It seems to me that 0002 is still not right. We can't fix the
> attcompression to whatever the default is at the time the index is
> created, because the default can be changed later, and there's no way
> to fix index afterward. I mean, it would be fine to do it that way if
> we were going to go with the other model, where the index state is
> separate from the table state, either can be changed independently,
> and it all gets dumped and restored. But, as it is, I think we should
> be deciding how to compress new values for an expression column based
> on the default_toast_compression setting at the time of compression,
> not the time of index creation.
>
Okay got it. Fixed as suggested.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From c478a311b3410d5360946e953e1bfd53bbef7197 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Wed, 24 Mar 2021 15:08:14 +0530
Subject: [PATCH v4 1/2] Show compression method in index describe
---
src/bin/psql/describe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index eeac0ef..9d15324 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1897,6 +1897,7 @@ describeOneTableDetails(const char *schemaname,
if (pset.sversion >= 140000 &&
!pset.hide_compression &&
(tableinfo.relkind == RELKIND_RELATION ||
+ tableinfo.relkind == RELKIND_INDEX ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE ||
tableinfo.relkind == RELKIND_MATVIEW))
{
--
1.8.3.1
From 75dbb3094973a4e69dafc213b760f5f13ab33a91 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Wed, 24 Mar 2021 14:02:06 +0530
Subject: [PATCH v4 2/2] Fix attcompression for index expression columns
For the expression columns, set the attcompression to the invalid
compression method and while actually compressing the data if the
attcompression is not valid then use the default compression method.
Basically, attcompression for the index attribute is always in sync with
that of the table attribute but for the expression columns that is not
possible so for them always use the current default method while
compressing the data.
---
src/backend/access/brin/brin_tuple.c | 8 +++++---
src/backend/access/common/indextuple.c | 16 ++++++++++++++--
src/backend/catalog/index.c | 9 +++++++++
src/test/regress/expected/compression.out | 6 ++++++
src/test/regress/expected/compression_1.out | 13 +++++++++++++
src/test/regress/sql/compression.sql | 7 +++++++
6 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 8d03e60..32ffd9f 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -220,10 +220,12 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
/*
* If the BRIN summary and indexed attribute use the same data
- * type, we can use the same compression method. Otherwise we
- * have to use the default method.
+ * type and it has a valid compression method, we can use the
+ * same compression method. Otherwise we have to use the
+ * default method.
*/
- if (att->atttypid == atttype->type_id)
+ if (att->atttypid == atttype->type_id &&
+ CompressionMethodIsValid(att->attcompression))
compression = att->attcompression;
else
compression = GetDefaultToastCompression();
diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 1f6b7b7..4d58644 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -103,8 +103,20 @@ index_form_tuple(TupleDesc tupleDescriptor,
(att->attstorage == TYPSTORAGE_EXTENDED ||
att->attstorage == TYPSTORAGE_MAIN))
{
- Datum cvalue = toast_compress_datum(untoasted_values[i],
- att->attcompression);
+ Datum cvalue;
+ char compression = att->attcompression;
+
+ /*
+ * If the compression method is not valid then use the default
+ * compression method. This can happen because, for the expression
+ * columns, we set the attcompression to the invalid compression
+ * method, while creating the index so that we can use the current
+ * default compression method when we actually need to compress.
+ */
+ if (!CompressionMethodIsValid(compression))
+ compression = GetDefaultToastCompression();
+
+ cvalue = toast_compress_datum(untoasted_values[i], compression);
if (DatumGetPointer(cvalue) != NULL)
{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 397d70d..5297d5e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -30,6 +30,7 @@
#include "access/relscan.h"
#include "access/sysattr.h"
#include "access/tableam.h"
+#include "access/toast_compression.h"
#include "access/transam.h"
#include "access/visibilitymap.h"
#include "access/xact.h"
@@ -379,6 +380,14 @@ ConstructTupleDescriptor(Relation heapRelation,
to->attalign = typeTup->typalign;
to->atttypmod = exprTypmod(indexkey);
+ /*
+ * For the expression column set attcompression to the invalid
+ * compression. If the value for this attribute needs to be
+ * compressed during insertion then it will be compressed using
+ * default compression.
+ */
+ to->attcompression = InvalidCompressionMethod;
+
ReleaseSysCache(tuple);
/*
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 566a187..61e97cb 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -313,6 +313,12 @@ SELECT pg_column_compression(f1) FROM cmdata;
lz4
(2 rows)
+-- test expression index
+DROP TABLE cmdata2;
+CREATE TABLE cmdata2 (f1 TEXT COMPRESSION pglz, f2 TEXT COMPRESSION lz4);
+CREATE UNIQUE INDEX idx1 ON cmdata2 ((f1 || f2));
+INSERT INTO cmdata2 VALUES((SELECT array_agg(md5(g::TEXT))::TEXT FROM
+generate_series(1, 50) g), VERSION());
-- 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 3990933..d03d625 100644
--- a/src/test/regress/expected/compression_1.out
+++ b/src/test/regress/expected/compression_1.out
@@ -309,6 +309,19 @@ SELECT pg_column_compression(f1) FROM cmdata;
pglz
(2 rows)
+-- test expression index
+DROP TABLE cmdata2;
+CREATE TABLE cmdata2 (f1 TEXT COMPRESSION pglz, f2 TEXT 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.
+CREATE UNIQUE INDEX idx1 ON cmdata2 ((f1 || f2));
+ERROR: relation "cmdata2" does not exist
+INSERT INTO cmdata2 VALUES((SELECT array_agg(md5(g::TEXT))::TEXT FROM
+generate_series(1, 50) g), VERSION());
+ERROR: relation "cmdata2" does not exist
+LINE 1: INSERT INTO cmdata2 VALUES((SELECT array_agg(md5(g::TEXT))::...
+ ^
-- 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 5e178be..76d1776 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -130,6 +130,13 @@ SELECT pg_column_compression(f1) FROM cmdata;
VACUUM FULL cmdata;
SELECT pg_column_compression(f1) FROM cmdata;
+-- test expression index
+DROP TABLE cmdata2;
+CREATE TABLE cmdata2 (f1 TEXT COMPRESSION pglz, f2 TEXT COMPRESSION lz4);
+CREATE UNIQUE INDEX idx1 ON cmdata2 ((f1 || f2));
+INSERT INTO cmdata2 VALUES((SELECT array_agg(md5(g::TEXT))::TEXT FROM
+generate_series(1, 50) g), VERSION());
+
-- check data is ok
SELECT length(f1) FROM cmdata;
SELECT length(f1) FROM cmdata1;
--
1.8.3.1
