On Fri, Jun 04, 2021 at 08:54:48AM +0900, Michael Paquier wrote: > I have done no recompression here, so I was just stressing the extra > test for the recompression. Sorry for the confusion.
I am not sure yet which way we are going, but cleaning up this code involves a couple of things: - Clean up the docs. - Update one of the tests of compression.sql, with its alternate output. - Clean up of reform_and_rewrite_tuple() where the rewrite is done. So that would give the attached. -- Michael
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index e2cd79ec54..1b8b640012 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -19,7 +19,6 @@
*/
#include "postgres.h"
-#include "access/detoast.h"
#include "access/genam.h"
#include "access/heapam.h"
#include "access/heaptoast.h"
@@ -27,7 +26,6 @@
#include "access/rewriteheap.h"
#include "access/syncscan.h"
#include "access/tableam.h"
-#include "access/toast_compression.h"
#include "access/tsmapi.h"
#include "access/xact.h"
#include "catalog/catalog.h"
@@ -2463,64 +2461,14 @@ reform_and_rewrite_tuple(HeapTuple tuple,
TupleDesc newTupDesc = RelationGetDescr(NewHeap);
HeapTuple copiedTuple;
int i;
- bool values_free[MaxTupleAttributeNumber];
-
- memset(values_free, 0, newTupDesc->natts * sizeof(bool));
heap_deform_tuple(tuple, oldTupDesc, values, isnull);
+ /* Be sure to null out any dropped columns */
for (i = 0; i < newTupDesc->natts; i++)
{
- /* Be sure to null out any dropped columns */
if (TupleDescAttr(newTupDesc, i)->attisdropped)
isnull[i] = true;
- else if (!isnull[i] && TupleDescAttr(newTupDesc, i)->attlen == -1)
- {
- /*
- * Use this opportunity to force recompression of any data that's
- * compressed with some TOAST compression method other than the
- * one configured for the column. We don't actually need to
- * perform the compression here; we just need to decompress. That
- * will trigger recompression later on.
- */
- struct varlena *new_value;
- ToastCompressionId cmid;
- char cmethod;
- char targetmethod;
-
- new_value = (struct varlena *) DatumGetPointer(values[i]);
- cmid = toast_get_compression_id(new_value);
-
- /* nothing to be done for uncompressed data */
- if (cmid == TOAST_INVALID_COMPRESSION_ID)
- continue;
-
- /* convert existing compression id to compression method */
- switch (cmid)
- {
- case TOAST_PGLZ_COMPRESSION_ID:
- cmethod = TOAST_PGLZ_COMPRESSION;
- break;
- case TOAST_LZ4_COMPRESSION_ID:
- cmethod = TOAST_LZ4_COMPRESSION;
- break;
- default:
- elog(ERROR, "invalid compression method id %d", cmid);
- cmethod = '\0'; /* keep compiler quiet */
- }
-
- /* figure out what the target method is */
- targetmethod = TupleDescAttr(newTupDesc, i)->attcompression;
- if (!CompressionMethodIsValid(targetmethod))
- targetmethod = default_toast_compression;
-
- /* if compression method doesn't match then detoast the value */
- if (targetmethod != cmethod)
- {
- values[i] = PointerGetDatum(detoast_attr(new_value));
- values_free[i] = true;
- }
- }
}
copiedTuple = heap_form_tuple(newTupDesc, values, isnull);
@@ -2528,13 +2476,6 @@ reform_and_rewrite_tuple(HeapTuple tuple,
/* The heap rewrite module does the rest */
rewrite_heap_tuple(rwstate, tuple, copiedTuple);
- /* Free any value detoasted previously */
- for (i = 0; i < newTupDesc->natts; i++)
- {
- if (values_free[i])
- pfree(DatumGetPointer(values[i]));
- }
-
heap_freetuple(copiedTuple);
}
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 5c645e4650..4c997e2602 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -297,7 +297,7 @@ SELECT pg_column_compression(f1) FROM cmpart2;
lz4
(2 rows)
---vacuum full to recompress the data
+-- VACUUM FULL does not recompress
SELECT pg_column_compression(f1) FROM cmdata;
pg_column_compression
-----------------------
@@ -309,7 +309,7 @@ VACUUM FULL cmdata;
SELECT pg_column_compression(f1) FROM cmdata;
pg_column_compression
-----------------------
- lz4
+ pglz
lz4
(2 rows)
diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out
index aac96037fc..15a23924ec 100644
--- a/src/test/regress/expected/compression_1.out
+++ b/src/test/regress/expected/compression_1.out
@@ -293,7 +293,7 @@ SELECT pg_column_compression(f1) FROM cmpart2;
-----------------------
(0 rows)
---vacuum full to recompress the data
+-- VACUUM FULL does not recompress
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 35557c1f7d..86332dcc51 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -126,7 +126,7 @@ INSERT INTO cmpart VALUES (repeat('123456789', 4004));
SELECT pg_column_compression(f1) FROM cmpart1;
SELECT pg_column_compression(f1) FROM cmpart2;
---vacuum full to recompress the data
+-- VACUUM FULL does not recompress
SELECT pg_column_compression(f1) FROM cmdata;
VACUUM FULL cmdata;
SELECT pg_column_compression(f1) FROM cmdata;
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 939d3fe273..c7a96811a1 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -394,11 +394,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
values inserted in future will be compressed (if the storage mode
permits compression at all).
This does not cause the table to be rewritten, so existing data may still
- be compressed with other compression methods. If the table is rewritten with
- <command>VACUUM FULL</command> or <command>CLUSTER</command>, or restored
+ be compressed with other compression methods. If the table is restored
with <application>pg_restore</application>, then all values are rewritten
with the configured compression method.
- However, when data is inserted from another relation (for example,
+ When data is inserted from another relation (for example,
by <command>INSERT ... SELECT</command>), values from the source table are
not necessarily detoasted, so any previously compressed data may retain
its existing compression method, rather than being recompressed with the
signature.asc
Description: PGP signature
