Optimize partial TOAST decompression
Hi, hackers!
I'm a student participating in GSoC 2019 and my project is related to TOAST
slices.
When I'm getting familiar with the postgresql codebase, I find that
PG_DETOAST_DATUM_SLICE, when to run on a compressed TOAST entry, will fetch
all compressed data chunks then extract the relevant slice. Obviously, this
is unnecessary, we only need to fetch the data chunks we need.
The patch optimizes partial TOAST decompression.
For an example of the improvement possible, this trivial example:
---------------------------------------------------------------------
create table slicingtest (
id serial primary key,
a text
);
insert into slicingtest (a) select
repeat('1234567890-=abcdefghijklmnopqrstuvwxyz', 1000000) as a from
generate_series(1,100);
\timing
select sum(length(substr(a, 0, 20))) from slicingtest;
---------------------------------------------------------------------
environment: Linux 4.15.0-33-generic #36~16.04.1-Ubuntu x86_64 GNU/Linux
On master, I get
Time: 28.123 ms (Take ten times average)
With the patch, I get
Time: 2.306 ms (take ten times average)
This seems to have a 10x improvement. If the number of toast data chunks is
more, I believe that patch can play a greater role, there are about 200
related TOAST data chunks for each entry in the case.
Related discussion:
/messages/by-id/CACowWR07EDm7Y4m2kbhN_jnys=BBf9A6768RyQdKm_=NpkcaWg@mail.gmail.com
Best regards, Binguo Bao.
Attachments:
0001-Optimize-partial-TOAST-decompression.patchtext/x-patch; charset=US-ASCII; name=0001-Optimize-partial-TOAST-decompression.patchDownload
From a7c99439ffe309526b57fe26ab367e4b7bf62f39 Mon Sep 17 00:00:00 2001
From: BBG <djydewang@gmail.com>
Date: Sun, 2 Jun 2019 19:18:46 +0800
Subject: [PATCH] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91..7d30538 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -273,8 +273,11 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, sliceoffset + slicelength + 1);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -2031,7 +2034,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2076,9 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2091,7 +2094,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
--
2.7.4
Hi, Binguo!
2 июня 2019 г., в 19:48, Binguo Bao <djydewang@gmail.com> написал(а):
Hi, hackers!
....
This seems to have a 10x improvement. If the number of toast data chunks is more, I believe that patch can play a greater role, there are about 200 related TOAST data chunks for each entry in the case.
That's really cool that you could produce meaningful patch long before end of GSoC!
I'll describe what is going on a little:
1. We have compressed value, which resides in TOAST table.
2. We want only some fraction of this value. We want some prefix with length L.
3. Previously Paul Ramsey submitted patch that omits decompression of value beyond desired L bytes.
4. Binguo's patch tries to do not fetch compressed data which will not bee needed to decompressor. In fact it fetches L bytes from TOAST table.
This is not correct: L bytes of compressed data do not always can be decoded into at least L bytes of data. At worst we have one control byte per 8 bytes of literal bytes. This means at most we need (L*9 + 8) / 8 bytes with current pglz format.
Also, I'm not sure you use SET_VARSIZE_COMPRESSED correctly...
Best regards, Andrey Borodin.
This is not correct: L bytes of compressed data do not always can be
decoded into at least L bytes of data. At worst we have one control byte
per 8 bytes of literal bytes. This means at most we need (L*9 + 8) / 8
bytes with current pglz format.
Good catch! I've corrected the related code in the patch.
Also, I'm not sure you use SET_VARSIZE_COMPRESSED correctly...
I followed the code in toast_fetch_datum function[1]https://github.com/postgres/postgres/blob/master/src/backend/access/heap/tuptoaster.c#L1898, and I didn't see any
wrong with it.
Best regards, Binguo Bao
[1]: https://github.com/postgres/postgres/blob/master/src/backend/access/heap/tuptoaster.c#L1898
https://github.com/postgres/postgres/blob/master/src/backend/access/heap/tuptoaster.c#L1898
Andrey Borodin <x4mmm@yandex-team.ru> 于2019年6月23日周日 下午5:23写道:
Show quoted text
Hi, Binguo!
2 июня 2019 г., в 19:48, Binguo Bao <djydewang@gmail.com> написал(а):
Hi, hackers!
....
This seems to have a 10x improvement. If the number of toast data chunks
is more, I believe that patch can play a greater role, there are about 200
related TOAST data chunks for each entry in the case.That's really cool that you could produce meaningful patch long before end
of GSoC!I'll describe what is going on a little:
1. We have compressed value, which resides in TOAST table.
2. We want only some fraction of this value. We want some prefix with
length L.
3. Previously Paul Ramsey submitted patch that omits decompression of
value beyond desired L bytes.
4. Binguo's patch tries to do not fetch compressed data which will not bee
needed to decompressor. In fact it fetches L bytes from TOAST table.This is not correct: L bytes of compressed data do not always can be
decoded into at least L bytes of data. At worst we have one control byte
per 8 bytes of literal bytes. This means at most we need (L*9 + 8) / 8
bytes with current pglz format.Also, I'm not sure you use SET_VARSIZE_COMPRESSED correctly...
Best regards, Andrey Borodin.
Attachments:
0001-Optimize-partial-TOAST-decompression-2.patchtext/x-patch; charset=US-ASCII; name=0001-Optimize-partial-TOAST-decompression-2.patchDownload
From 505dcc4fdef18710c98718685180190056b4d9ed Mon Sep 17 00:00:00 2001
From: BBG <djydewang@gmail.com>
Date: Sun, 2 Jun 2019 19:18:46 +0800
Subject: [PATCH] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91..89ffc2d 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -262,6 +262,8 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
struct varlena *result;
char *attrdata;
int32 attrsize;
+ int32 needsize;
+ int32 max_compressed_size;
if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
@@ -273,8 +275,22 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ needsize = sliceoffset + slicelength;
+ if (needsize > (INT_MAX / 9))
+ {
+ /* Approximate to avoid overflow */
+ max_compressed_size = (needsize / 8 + 1) * 9;
+ }
+ else
+ {
+ max_compressed_size = (needsize * 9) / 8 + 1;
+ }
+
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, max_compressed_size);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -2031,7 +2047,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2089,9 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2091,7 +2107,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
--
2.7.4
Hi!
Please, do not use top-posting, i.e. reply style where you quote whole message under your response. It makes reading of archives terse.
24 июня 2019 г., в 7:53, Binguo Bao <djydewang@gmail.com> написал(а):
This is not correct: L bytes of compressed data do not always can be decoded into at least L bytes of data. At worst we have one control byte per 8 bytes of literal bytes. This means at most we need (L*9 + 8) / 8 bytes with current pglz format.
Good catch! I've corrected the related code in the patch.
...
<0001-Optimize-partial-TOAST-decompression-2.patch>
I've took a look into the code.
I think we should extract function for computation of max_compressed_size and put it somewhere along with pglz code. Just in case something will change something about pglz so that they would not forget about compression algorithm assumption.
Also I suggest just using 64 bit computation to avoid overflows. And I think it worth to check if max_compressed_size is whole data and use min of (max_compressed_size, uncompressed_data_size).
Also you declared needsize and max_compressed_size too far from use. But this will be solved by function extraction anyway.
Thanks!
Best regards, Andrey Borodin.
Hi!
Andrey Borodin <x4mmm@yandex-team.ru> 于2019年6月29日周六 下午9:48写道:
Hi!
Please, do not use top-posting, i.e. reply style where you quote whole
message under your response. It makes reading of archives terse.24 июня 2019 г., в 7:53, Binguo Bao <djydewang@gmail.com> написал(а):
This is not correct: L bytes of compressed data do not always can be
decoded into at least L bytes of data. At worst we have one control byte
per 8 bytes of literal bytes. This means at most we need (L*9 + 8) / 8
bytes with current pglz format.Good catch! I've corrected the related code in the patch.
...
<0001-Optimize-partial-TOAST-decompression-2.patch>I've took a look into the code.
I think we should extract function for computation of max_compressed_size
and put it somewhere along with pglz code. Just in case something will
change something about pglz so that they would not forget about compression
algorithm assumption.Also I suggest just using 64 bit computation to avoid overflows. And I
think it worth to check if max_compressed_size is whole data and use min of
(max_compressed_size, uncompressed_data_size).Also you declared needsize and max_compressed_size too far from use. But
this will be solved by function extraction anyway.Thanks!
Best regards, Andrey Borodin.
Thanks for the suggestion.
I've extracted function for computation for max_compressed_size and put the
function into pg_lzcompress.c.
Best regards, Binguo Bao.
Attachments:
0001-Optimize-partial-TOAST-decompression-3.patchtext/x-patch; charset=US-ASCII; name=0001-Optimize-partial-TOAST-decompression-3.patchDownload
From 79a1b4c292a0629df9d7ba3dc04e879aadca7a61 Mon Sep 17 00:00:00 2001
From: BBG <djydewang@gmail.com>
Date: Sun, 2 Jun 2019 19:18:46 +0800
Subject: [PATCH] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 24 +++++++++++++++++-------
src/common/pg_lzcompress.c | 22 ++++++++++++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91..684f1b2 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -266,6 +266,7 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
struct varatt_external toast_pointer;
+ int32 max_size;
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
@@ -273,8 +274,13 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
+ toast_pointer.va_rawsize);
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -2031,7 +2037,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2079,9 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2091,7 +2097,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 988b398..2b5f112 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -771,3 +771,25 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
+
+
+
+/* ----------
+ * pglz_max_compressed_size -
+ *
+ * Calculate the maximum size of the compressed slice corresponding to the
+ * raw slice. Return the maximum size, or raw size if maximum size is greater
+ * than raw size.
+ * ----------
+ */
+int32
+pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size)
+{
+ int32 result;
+
+ /*
+ * Use int64 to prevent overflow during calculation.
+ */
+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+ return result > raw_size ? raw_size : result;
+}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index 5555764..cda3e1d 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.7.4
On Mon, Jul 1, 2019 at 6:46 AM Binguo Bao <djydewang@gmail.com> wrote:
Andrey Borodin <x4mmm@yandex-team.ru> 于2019年6月29日周六 下午9:48写道:
I've took a look into the code.
I think we should extract function for computation of max_compressed_size and put it somewhere along with pglz code. Just in case something will change something about pglz so that they would not forget about compression algorithm assumption.Also I suggest just using 64 bit computation to avoid overflows. And I think it worth to check if max_compressed_size is whole data and use min of (max_compressed_size, uncompressed_data_size).
Also you declared needsize and max_compressed_size too far from use. But this will be solved by function extraction anyway.
Thanks for the suggestion.
I've extracted function for computation for max_compressed_size and put the function into pg_lzcompress.c.
This looks good to me. A little commentary around why
pglz_maximum_compressed_size() returns a universally correct answer
(there's no way the compressed size can ever be larger than this
because...) would be nice for peasants like myself.
If you're looking to continue down this code line in your next patch,
the next TODO item is a little more involved: a user-land (ala
PG_DETOAST_DATUM) iterator API for access of TOAST datums would allow
the optimization of searching of large objects like JSONB types, and
so on, where the thing you are looking for is not at a known location
in the object. So, things like looking for a particular substring in a
string, or looking for a particular key in a JSONB. "Iterate until you
find the thing." would allow optimization of some code lines that
currently require full decompression of the objects.
P.
Paul Ramsey <pramsey@cleverelephant.ca> 于2019年7月2日周二 下午10:46写道:
This looks good to me. A little commentary around why
pglz_maximum_compressed_size() returns a universally correct answer
(there's no way the compressed size can ever be larger than this
because...) would be nice for peasants like myself.If you're looking to continue down this code line in your next patch,
the next TODO item is a little more involved: a user-land (ala
PG_DETOAST_DATUM) iterator API for access of TOAST datums would allow
the optimization of searching of large objects like JSONB types, and
so on, where the thing you are looking for is not at a known location
in the object. So, things like looking for a particular substring in a
string, or looking for a particular key in a JSONB. "Iterate until you
find the thing." would allow optimization of some code lines that
currently require full decompression of the objects.P.
Thanks for your comment. I've updated the patch.
As for the iterator API, I've implemented a de-TOAST iterator actually[0]/messages/by-id/CAL-OGks_onzpc9M9bXPCztMofWULcFkyeCeKiAgXzwRL8kXiag@mail.gmail.com.
And I’m looking for more of its application scenarios and perfecting it.
Any comments would be much appreciated.
Best Regards, Binguo Bao.
[0]: /messages/by-id/CAL-OGks_onzpc9M9bXPCztMofWULcFkyeCeKiAgXzwRL8kXiag@mail.gmail.com
/messages/by-id/CAL-OGks_onzpc9M9bXPCztMofWULcFkyeCeKiAgXzwRL8kXiag@mail.gmail.com
Attachments:
0001-Optimize-partial-TOAST-decompression-4.patchtext/x-patch; charset=US-ASCII; name=0001-Optimize-partial-TOAST-decompression-4.patchDownload
From 2e4e2838937ec6fa1404fe529e7ed303e391d1b2 Mon Sep 17 00:00:00 2001
From: BBG <djydewang@gmail.com>
Date: Sun, 2 Jun 2019 19:18:46 +0800
Subject: [PATCH] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 24 +++++++++++++++++-------
src/common/pg_lzcompress.c | 26 ++++++++++++++++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91..684f1b2 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -266,6 +266,7 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
struct varatt_external toast_pointer;
+ int32 max_size;
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
@@ -273,8 +274,13 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
+ toast_pointer.va_rawsize);
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -2031,7 +2037,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2079,9 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2091,7 +2097,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 988b398..80ed17a 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -771,3 +771,29 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
+
+
+
+/* ----------
+ * pglz_max_compressed_size -
+ *
+ * Calculate the maximum size of the compressed slice corresponding to the
+ * raw slice. Return the maximum size, or raw size if maximum size is larger
+ * than raw size.
+ * ----------
+ */
+int32
+pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size)
+{
+ int32 result;
+
+ /*
+ * Use int64 to prevent overflow during calculation.
+ */
+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+
+ /*
+ * Note that compressed size will never be larger than raw size.
+ */
+ return result > raw_size ? raw_size : result;
+}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index 5555764..cda3e1d 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.7.4
3 июля 2019 г., в 18:06, Binguo Bao <djydewang@gmail.com> написал(а):
Paul Ramsey <pramsey@cleverelephant.ca> 于2019年7月2日周二 下午10:46写道:
This looks good to me. A little commentary around why
pglz_maximum_compressed_size() returns a universally correct answer
(there's no way the compressed size can ever be larger than this
because...) would be nice for peasants like myself.
...Thanks for your comment. I've updated the patch.
Thanks Biguo and Paul! From my POV patch looks ready for committer, so I switched state on CF.
Best regards, Andrey Borodin.
On Thu, Jul 04, 2019 at 11:10:24AM +0200, Andrey Borodin wrote:
3 июля 2019 г., в 18:06, Binguo Bao <djydewang@gmail.com> написал(а):
Paul Ramsey <pramsey@cleverelephant.ca> 于2019年7月2日周二 下午10:46写道:
This looks good to me. A little commentary around why
pglz_maximum_compressed_size() returns a universally correct answer
(there's no way the compressed size can ever be larger than this
because...) would be nice for peasants like myself.
...Thanks for your comment. I've updated the patch.
Thanks Biguo and Paul! From my POV patch looks ready for committer, so I switched state on CF.
I've done a bit of testing and benchmaring on this patch today, and
there's a bug somewhere, making it look like there are corrupted data.
What I'm seeing is this:
CREATE TABLE t (a text);
-- attached is data for one row
COPY t FROM '/tmp/t.data';
SELECT length(substr(a,1000)) from t;
psql: ERROR: compressed data is corrupted
SELECT length(substr(a,0,1000)) from t;
length
--------
999
(1 row)
SELECT length(substr(a,1000)) from t;
psql: ERROR: invalid memory alloc request size 2018785106
That's quite bizarre behavior - it does work with a prefix, but not with
suffix. And the exact ERROR changes after the prefix query. (Of course,
on master it works in all cases.)
The backtrace (with the patch applied) looks like this:
#0 toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2291
#1 toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2277
#2 0x00000000004c3b08 in heap_tuple_untoast_attr_slice (attr=<optimized out>, sliceoffset=0, slicelength=-1) at tuptoaster.c:315
#3 0x000000000085c1e5 in pg_detoast_datum_slice (datum=<optimized out>, first=<optimized out>, count=<optimized out>) at fmgr.c:1767
#4 0x0000000000833b7a in text_substring (str=133761519127512, start=0, length=<optimized out>, length_not_specified=<optimized out>) at varlena.c:956
...
I've only observed this with a very small number of rows (the data is
generated randomly with different compressibility etc.), so I'm only
attaching one row that exhibits this issue.
My guess is toast_fetch_datum_slice() gets confused by the headers or
something, or something like that. FWIW the new code added to this
function does not adhere to our code style, and would deserve some
additional explanation of what it's doing/why. Same for the
heap_tuple_untoast_attr_slice, BTW.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Of course, I forgot to attach the files, so here they are.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> 于2019年7月5日周五 上午1:46写道:
I've done a bit of testing and benchmaring on this patch today, and
there's a bug somewhere, making it look like there are corrupted data.What I'm seeing is this:
CREATE TABLE t (a text);
-- attached is data for one row
COPY t FROM '/tmp/t.data';SELECT length(substr(a,1000)) from t;
psql: ERROR: compressed data is corruptedSELECT length(substr(a,0,1000)) from t;
length
--------
999
(1 row)SELECT length(substr(a,1000)) from t;
psql: ERROR: invalid memory alloc request size 2018785106That's quite bizarre behavior - it does work with a prefix, but not with
suffix. And the exact ERROR changes after the prefix query. (Of course,
on master it works in all cases.)The backtrace (with the patch applied) looks like this:
#0 toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2291
#1 toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2277
#2 0x00000000004c3b08 in heap_tuple_untoast_attr_slice (attr=<optimized
out>, sliceoffset=0, slicelength=-1) at tuptoaster.c:315
#3 0x000000000085c1e5 in pg_detoast_datum_slice (datum=<optimized out>,
first=<optimized out>, count=<optimized out>) at fmgr.c:1767
#4 0x0000000000833b7a in text_substring (str=133761519127512, start=0,
length=<optimized out>, length_not_specified=<optimized out>) at
varlena.c:956
...I've only observed this with a very small number of rows (the data is
generated randomly with different compressibility etc.), so I'm only
attaching one row that exhibits this issue.My guess is toast_fetch_datum_slice() gets confused by the headers or
something, or something like that. FWIW the new code added to this
function does not adhere to our code style, and would deserve some
additional explanation of what it's doing/why. Same for the
heap_tuple_untoast_attr_slice, BTW.regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, Tomas!
Thanks for your testing and the suggestion.
That's quite bizarre behavior - it does work with a prefix, but not with
suffix. And the exact ERROR changes after the prefix query.
I think bug is caused by "#2 0x00000000004c3b08 in
heap_tuple_untoast_attr_slice (attr=<optimized out>, sliceoffset=0,
slicelength=-1) at tuptoaster.c:315",
since I ignore the case where slicelength is negative, and I've appended
some comments for heap_tuple_untoast_attr_slice for the case.
FWIW the new code added to this
function does not adhere to our code style, and would deserve some
additional explanation of what it's doing/why. Same for the
heap_tuple_untoast_attr_slice, BTW.
I've added more comments to explain the code's behavior.
Besides, I also modified the macro "TOAST_COMPRESS_RAWDATA" to
"TOAST_COMPRESS_DATA" since
it is used to get toast compressed data rather than raw data.
Best Regards, Binguo Bao.
Attachments:
0001-Optimize-partial-TOAST-decompression-5.patchtext/x-patch; charset=US-ASCII; name=0001-Optimize-partial-TOAST-decompression-5.patchDownload
From bcf04278b4d5956cd5f5fdab0d954b36adfd0022 Mon Sep 17 00:00:00 2001
From: BBG <djydewang@gmail.com>
Date: Sun, 2 Jun 2019 19:18:46 +0800
Subject: [PATCH] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 57 ++++++++++++++++++++++++++++--------
src/common/pg_lzcompress.c | 26 ++++++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91..1eb6cca 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -61,7 +61,8 @@ typedef struct toast_compress_header
*/
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
-#define TOAST_COMPRESS_RAWDATA(ptr) \
+#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
+#define TOAST_COMPRESS_DATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -252,6 +253,8 @@ heap_tuple_untoast_attr(struct varlena *attr)
*
* Public entry point to get back part of a toasted value
* from compression or external storage.
+ *
+ * Note if slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -273,8 +276,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ /*
+ * If only need the prefix slice, fetch enough datums to decompress.
+ * Otherwise, fetch all datums.
+ */
+ if (slicelength > 0 && sliceoffset >= 0)
+ {
+ int32 max_size;
+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
+ TOAST_COMPRESS_SIZE(attr));
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, max_size);
+ }
+ else
+ preslice = toast_fetch_datum(attr);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -1391,7 +1409,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_RAWDATA(tmp),
+ TOAST_COMPRESS_DATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2031,7 +2049,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2091,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum,
+ * since we can decompress the prefix raw data from it.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2086,12 +2105,26 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
+ {
+ /*
+ * If we are going to fetch the compressed external datum slice
+ * at the start of the object, also should fetch rawsize field used
+ * to record the size of the raw data.
+ */
+ length = length + sizeof(int32);
+ }
+
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2274,8 +2307,8 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
- VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
+ if (pglz_decompress(TOAST_COMPRESS_DATA(attr),
+ TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
elog(ERROR, "compressed data is corrupted");
@@ -2301,7 +2334,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
+ rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 988b398..ac7d66d 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -771,3 +771,29 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
+
+
+
+/* ----------
+ * pglz_max_compressed_size -
+ *
+ * Calculate the maximum size of the compressed slice corresponding to the
+ * raw slice. Return the maximum size, or total compressed size if maximum
+ * size is larger than total compressed size.
+ * ----------
+ */
+int32
+pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)
+{
+ int32 result;
+
+ /*
+ * Use int64 to prevent overflow during calculation.
+ */
+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+
+ /*
+ * Note that slice compressed size will never be larger than total compressed size.
+ */
+ return result > total_compressed_size ? total_compressed_size: result;
+}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index 5555764..cda3e1d 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.7.4
On Sat, Jul 06, 2019 at 02:27:56AM +0800, Binguo Bao wrote:
Hi, Tomas!
Thanks for your testing and the suggestion.That's quite bizarre behavior - it does work with a prefix, but not with
suffix. And the exact ERROR changes after the prefix query.
I think bug is caused by "#2 0x00000000004c3b08 in
heap_tuple_untoast_attr_slice (attr=<optimized out>, sliceoffset=0,
slicelength=-1) at tuptoaster.c:315",
since I ignore the case where slicelength is negative, and I've appended
some comments for heap_tuple_untoast_attr_slice for the case.FWIW the new code added to this
function does not adhere to our code style, and would deserve some
additional explanation of what it's doing/why. Same for the
heap_tuple_untoast_attr_slice, BTW.I've added more comments to explain the code's behavior.
Besides, I also modified the macro "TOAST_COMPRESS_RAWDATA" to
"TOAST_COMPRESS_DATA" since
it is used to get toast compressed data rather than raw data.
Thanks, this seems to address the issue - I can no longer reproduce the
crashes, allowing the benchmark to complete. I'm attaching the script I
used and spreadsheet with a summary of results.
For the cases I've tested (100k - 10M values, different compressibility,
different prefix/length values), the results are kinda mixed - many
cases got much faster (~2x), but other cases got slower too. We're
however talking about queries taking a couple of miliseconds, so in
absolute numbers the differences are small.
That does not mean the optimization is useless - but the example shared
at the beginning of this thread is quite extreme, as the values are
extremely compressible. Each value is ~38MB (in plaintext), but a table
with 100 such values has only ~40MB. Tha's 100:1 compression ratio,
which I think is not typical for real-world data sets.
The data I've used are less extreme, depending on the fraction of random
data in values.
I went through the code too. I've reworder a couple of comments and code
style issues, but there are a couple of more serious issues.
1) Why rename TOAST_COMPRESS_RAWDATA to TOAST_COMPRESS_DATA?
This seems unnecessary, and it discards the clear hint that it's about
accessing the *raw* data, and the relation to TOAST_COMPRESS_RAWSIZE.
IMHO we should keep the original naming.
2) pglz_maximum_compressed_size signatures are confusing
There are two places with pglz_maximum_compressed_size signature, and
those places are kinda out of sync when it comes to parameter names:
int32
pglz_maximum_compressed_size(int32 raw_slice_size,
int32 total_compressed_size)
extern
int32 pglz_maximum_compressed_size(int32 raw_slice_size,
int32 raw_size);
Also, pg_lzcompress.c has no concept of a "slice" because it only deals
with simple compression, slicing is responsibility of the tuptoaster. So
we should not mix those two, not even in comments.
I propose tweaks per the attached patch - I think it makes the code
clearer, and it's mostly cosmetic stuff. But I haven't tested the
changes beyond "it compiles".
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Optimize-partial-TOAST-decompression.patchtext/plain; charset=us-asciiDownload
From d50ae41bf1d1c4e2786a55a610300606c1ec43a5 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 15:41:37 +0200
Subject: [PATCH 1/2] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 57 ++++++++++++++++++++++------
src/common/pg_lzcompress.c | 26 +++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 55d6e91d1c..1eb6ccaca2 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -61,7 +61,8 @@ typedef struct toast_compress_header
*/
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
-#define TOAST_COMPRESS_RAWDATA(ptr) \
+#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
+#define TOAST_COMPRESS_DATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -252,6 +253,8 @@ heap_tuple_untoast_attr(struct varlena *attr)
*
* Public entry point to get back part of a toasted value
* from compression or external storage.
+ *
+ * Note if slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -273,8 +276,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */
- preslice = toast_fetch_datum(attr);
+ /*
+ * If only need the prefix slice, fetch enough datums to decompress.
+ * Otherwise, fetch all datums.
+ */
+ if (slicelength > 0 && sliceoffset >= 0)
+ {
+ int32 max_size;
+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
+ TOAST_COMPRESS_SIZE(attr));
+ /*
+ * Be sure to get enough compressed slice
+ * and compressed marker will get set automatically
+ */
+ preslice = toast_fetch_datum_slice(attr, 0, max_size);
+ }
+ else
+ preslice = toast_fetch_datum(attr);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -1391,7 +1409,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_RAWDATA(tmp),
+ TOAST_COMPRESS_DATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2031,7 +2049,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.
+ * Note that this function supports non-compressed external datums
+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2091,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*
- * we can't return a compressed datum which is meaningful to toast later
+ * It's meaningful to fetch slices at the start of a compressed datum,
+ * since we can decompress the prefix raw data from it.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2086,12 +2105,26 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
+ {
+ /*
+ * If we are going to fetch the compressed external datum slice
+ * at the start of the object, also should fetch rawsize field used
+ * to record the size of the raw data.
+ */
+ length = length + sizeof(int32);
+ }
+
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
+ } else {
+ SET_VARSIZE(result, length + VARHDRSZ);
+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2274,8 +2307,8 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
- VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
+ if (pglz_decompress(TOAST_COMPRESS_DATA(attr),
+ TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
elog(ERROR, "compressed data is corrupted");
@@ -2301,7 +2334,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
+ rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 988b3987d0..ac7d66db77 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -771,3 +771,29 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
+
+
+
+/* ----------
+ * pglz_max_compressed_size -
+ *
+ * Calculate the maximum size of the compressed slice corresponding to the
+ * raw slice. Return the maximum size, or total compressed size if maximum
+ * size is larger than total compressed size.
+ * ----------
+ */
+int32
+pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)
+{
+ int32 result;
+
+ /*
+ * Use int64 to prevent overflow during calculation.
+ */
+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+
+ /*
+ * Note that slice compressed size will never be larger than total compressed size.
+ */
+ return result > total_compressed_size ? total_compressed_size: result;
+}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index 555576436c..cda3e1d364 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
0002-review-reworks.patchtext/plain; charset=us-asciiDownload
From 34a17fd6df4f531527b976df043aed6fb5b6add0 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 16:39:17 +0200
Subject: [PATCH 2/2] review reworks
---
src/backend/access/heap/tuptoaster.c | 47 +++++++++++++++-------------
src/common/pg_lzcompress.c | 19 +++++------
src/include/common/pg_lzcompress.h | 2 +-
3 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 1eb6ccaca2..809a52196c 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -62,7 +62,7 @@ typedef struct toast_compress_header
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
-#define TOAST_COMPRESS_DATA(ptr) \
+#define TOAST_COMPRESS_RAWDATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -254,7 +254,7 @@ heap_tuple_untoast_attr(struct varlena *attr)
* Public entry point to get back part of a toasted value
* from compression or external storage.
*
- * Note if slicelength is negative, return suffix of the value.
+ * Note: when slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -277,17 +277,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
/*
- * If only need the prefix slice, fetch enough datums to decompress.
- * Otherwise, fetch all datums.
+ * When a prefix of the value is requested, fetch enough slices to
+ * decompress. Otherwise, fetch all slices.
*/
if (slicelength > 0 && sliceoffset >= 0)
{
int32 max_size;
+
+ /*
+ * Determine maximum amount of compressed data needed for a prefix
+ * of a given length (after decompression).
+ */
max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
TOAST_COMPRESS_SIZE(attr));
+
/*
- * Be sure to get enough compressed slice
- * and compressed marker will get set automatically
+ * Fetch enough compressed slices, compressed marker will get set
+ * automatically.
*/
preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
@@ -1409,7 +1415,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_DATA(tmp),
+ TOAST_COMPRESS_RAWDATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2050,7 +2056,8 @@ toast_fetch_datum(struct varlena *attr)
* in the toast relation
*
* Note that this function supports non-compressed external datums
- * and compressed external datum slices at the start of the object.
+ * and compressed external datums (in which case the requrested slice
+ * has to be a prefix, i.e. sliceoffset has to be 0).
* ----------
*/
static struct varlena *
@@ -2092,7 +2099,8 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
/*
* It's meaningful to fetch slices at the start of a compressed datum,
- * since we can decompress the prefix raw data from it.
+ * since we can decompress the prefix raw data without decompressing the
+ * whole value.
*/
Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
@@ -2105,26 +2113,23 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ /*
+ * When fetching a prefix of a compressed external datum, account for
+ * the rawsize tracking amount of raw data (it's stored at the beginning
+ * as an int32 value).
+ */
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
- {
- /*
- * If we are going to fetch the compressed external datum slice
- * at the start of the object, also should fetch rawsize field used
- * to record the size of the raw data.
- */
length = length + sizeof(int32);
- }
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
- } else {
+ else
SET_VARSIZE(result, length + VARHDRSZ);
- }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2307,7 +2312,7 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_DATA(attr),
+ if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
@@ -2334,7 +2339,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
+ rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index ac7d66db77..18766ade02 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -773,27 +773,28 @@ pglz_decompress(const char *source, int32 slen, char *dest,
}
-
/* ----------
* pglz_max_compressed_size -
*
- * Calculate the maximum size of the compressed slice corresponding to the
- * raw slice. Return the maximum size, or total compressed size if maximum
- * size is larger than total compressed size.
+ * Calculate the maximum compressed size for a given amount of raw data.
+ * Return the maximum size, or total compressed size if maximum size is
+ * larger than total compressed size.
* ----------
*/
int32
-pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)
+pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size)
{
- int32 result;
+ int32 compressed_size;
/*
* Use int64 to prevent overflow during calculation.
*/
- result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+ compressed_size = (int32) ((int64) rawsize * 9 + 8) / 8;
/*
- * Note that slice compressed size will never be larger than total compressed size.
+ * Maximum compressed size can't be larger than total compressed size.
*/
- return result > total_compressed_size ? total_compressed_size: result;
+ compressed_size = Min(compressed_size, total_compressed_size);
+
+ return compressed_size;
}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index cda3e1d364..e59fe2eaea 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,6 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
-extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
+extern int32 pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
comparison.odsapplication/vnd.oasis.opendocument.spreadsheetDownload
PK bi�N�l9�. . mimetypeapplication/vnd.oasis.opendocument.spreadsheetPK bi�N��_��"