pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedRe: [HACKERS] Custom compression methods (mac+lz4.h)
21+ messages / 4 participants
[nested] [flat]
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
@ 2021-03-20 21:23 Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Justin Pryzby @ 2021-03-20 21:23 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers@lists.postgresql.org
On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote:
> On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > Working with one of Andrey's patches on another thread, he reported offlist
> > getting this message, resolved by this patch. Do you see this warning during
> > ./configure ? The latest CI is of a single patch without the LZ4 stuff, so I
> > can't check its log.
> >
> > configure: WARNING: lz4.h: accepted by the compiler, rejected by the preprocessor!
> > configure: WARNING: lz4.h: proceeding with the compiler's result
>
> No, I don't see this. I wonder whether this could possibly be an
> installation issue on Andrey's machine? If not, it must be
> version-dependent or installation-dependent in some way.
Andrey, can you check if latest HEAD (bbe0a81db) has these ./configure warnings ?
If so, can you check if your environment is clean, specifically lz4.h - if
you've installed LZ4 from source, I have to imagine that's relevant.
Most users won't do that, but it should be a supported configuration, too.
--
Justin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
@ 2021-03-20 21:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-20 21:37 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers@lists.postgresql.org
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote:
>> On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>>> configure: WARNING: lz4.h: accepted by the compiler, rejected by the preprocessor!
>>> configure: WARNING: lz4.h: proceeding with the compiler's result
>> No, I don't see this. I wonder whether this could possibly be an
>> installation issue on Andrey's machine? If not, it must be
>> version-dependent or installation-dependent in some way.
> Andrey, can you check if latest HEAD (bbe0a81db) has these ./configure warnings ?
FWIW, I also saw that, when building HEAD against MacPorts' version
of liblz4 on an M1 Mac. config.log has
configure:13536: checking lz4.h usability
configure:13536: ccache clang -c -I/opt/local/include -Wall -Wmissing-prototype\
s -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmi\
ssing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unus\
ed-command-line-argument -g -O2 -isysroot /Applications/Xcode.app/Contents/Deve\
loper/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk conftest.c >&5
configure:13536: $? = 0
configure:13536: result: yes
configure:13536: checking lz4.h presence
configure:13536: ccache clang -E -isysroot /Applications/Xcode.app/Contents/Dev\
eloper/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk conftest.c
conftest.c:67:10: fatal error: 'lz4.h' file not found
#include <lz4.h>
^~~~~~~
1 error generated.
configure:13536: $? = 1
Digging around, it looks like the "-I/opt/local/include" bit came
from LZ4_CFLAGS, which we then stuck into CFLAGS, but it needed
to be put in CPPFLAGS in order to make this test work.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-20 23:19 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Justin Pryzby @ 2021-03-20 23:19 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers@lists.postgresql.org
On Sat, Mar 20, 2021 at 05:37:07PM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote:
> >> On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >>> configure: WARNING: lz4.h: accepted by the compiler, rejected by the preprocessor!
> >>> configure: WARNING: lz4.h: proceeding with the compiler's result
>
> >> No, I don't see this. I wonder whether this could possibly be an
> >> installation issue on Andrey's machine? If not, it must be
> >> version-dependent or installation-dependent in some way.
>
> > Andrey, can you check if latest HEAD (bbe0a81db) has these ./configure warnings ?
>
> FWIW, I also saw that, when building HEAD against MacPorts' version
> of liblz4 on an M1 Mac. config.log has
...
> Digging around, it looks like the "-I/opt/local/include" bit came
> from LZ4_CFLAGS, which we then stuck into CFLAGS, but it needed
> to be put in CPPFLAGS in order to make this test work.
If it's the same as the issue Andrey reported, then it causes a ./configure
WARNING, which is resolved by the ac_save hack, which I copied from ICU.
I'll shortly send a patchset including my tentative fix for that.
The configure.ac bits are also on this other thread:
https://www.postgresql.org/message-id/20210315180918.GW29463%40telsasoft.com
0005-re-add-wal_compression_method-lz4.patch
+if test "$with_lz4" = yes; then
+ ac_save_CPPFLAGS=$CPPFLAGS
+ CPPFLAGS="$LZ4_CFLAGS $CPPFLAGS"
+
+ # Verify we have LZ4's header files
+ AC_CHECK_HEADERS(lz4/lz4.h, [],
+ [AC_CHECK_HEADERS(lz4.h, [], [AC_MSG_ERROR([lz4.h header file is required for LZ4])])])
+
+ CPPFLAGS=$ac_save_CPPFLAGS
+fi
--
Justin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
@ 2021-03-21 00:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-21 00:44 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers@lists.postgresql.org
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sat, Mar 20, 2021 at 05:37:07PM -0400, Tom Lane wrote:
>> Digging around, it looks like the "-I/opt/local/include" bit came
>> from LZ4_CFLAGS, which we then stuck into CFLAGS, but it needed
>> to be put in CPPFLAGS in order to make this test work.
> If it's the same as the issue Andrey reported, then it causes a ./configure
> WARNING, which is resolved by the ac_save hack, which I copied from ICU.
I think probably what we need to do, rather than shove the pkg-config
results willy-nilly into our flags, is to disassemble them like we do
with the same results for xml2. If you ask me, the way we are handling
ICU flags is a poor precedent that is going to blow up at some point;
the only reason it hasn't is that people aren't building --with-icu that
much yet.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 01:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 07:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Tom Lane @ 2021-03-21 01:33 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers@lists.postgresql.org
BTW, I tried doing "make installcheck" after having adjusted
default_toast_compression to be "lz4". The compression test
itself fails because it's expecting the other setting; that
ought to be made more robust. Also, I see some diffs in the
indirect_toast test, which seems perhaps worthy of investigation.
(The diffs look to be just row ordering, but why?)
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 03:40 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Dilip Kumar @ 2021-03-21 03:40 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, Mar 21, 2021 at 7:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> BTW, I tried doing "make installcheck" after having adjusted
> default_toast_compression to be "lz4". The compression test
> itself fails because it's expecting the other setting; that
> ought to be made more robust.
Yeah, we need to set the default_toast_compression in the beginning of
the test as attached.
Also, I see some diffs in the
> indirect_toast test, which seems perhaps worthy of investigation.
> (The diffs look to be just row ordering, but why?)
I will look into this.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
0001-fix-compression-test.patch (application/octet-stream, 1.9K) download
From d65f952287b443e95e631253430340e3f9cb5b2b Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Sun, 21 Mar 2021 09:08:47 +0530
Subject: [PATCH] fix compression test
---
src/test/regress/expected/compression.out | 2 ++
src/test/regress/sql/compression.sql | 3 +++
2 files changed, 5 insertions(+)
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 3de2886..15af027 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -1,4 +1,5 @@
\set HIDE_TOAST_COMPRESSION false
+SET default_toast_compression = 'pglz';
-- test creating table with compression method
CREATE TABLE cmdata(f1 text COMPRESSION pglz);
CREATE INDEX idx ON cmdata(f1);
@@ -245,6 +246,7 @@ CREATE TABLE cmdata2 (f1 text);
--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
f1 | text | | | | extended | lz4 | |
+SET default_toast_compression = 'pglz';
-- test alter compression method
ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4;
INSERT INTO cmdata VALUES (repeat('123456789', 4004));
diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql
index d97e26b..1280fa4 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -1,5 +1,7 @@
\set HIDE_TOAST_COMPRESSION false
+SET default_toast_compression = 'pglz';
+
-- test creating table with compression method
CREATE TABLE cmdata(f1 text COMPRESSION pglz);
CREATE INDEX idx ON cmdata(f1);
@@ -100,6 +102,7 @@ SET default_toast_compression = 'lz4';
DROP TABLE cmdata2;
CREATE TABLE cmdata2 (f1 text);
\d+ cmdata2
+SET default_toast_compression = 'pglz';
-- test alter compression method
ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4;
--
1.8.3.1
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
@ 2021-03-21 05:22 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Dilip Kumar @ 2021-03-21 05:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, Mar 21, 2021 at 9:10 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Sun, Mar 21, 2021 at 7:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > BTW, I tried doing "make installcheck" after having adjusted
> > default_toast_compression to be "lz4". The compression test
> > itself fails because it's expecting the other setting; that
> > ought to be made more robust.
>
> Yeah, we need to set the default_toast_compression in the beginning of
> the test as attached.
In the last patch, I did not adjust the compression_1.out so fixed
that in the attached patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v1-0001-fix-compression-test.patch (application/octet-stream, 2.8K) download
From 46f87e52c1b1de903977480838d346e37d1be567 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Sun, 21 Mar 2021 09:08:47 +0530
Subject: [PATCH v1] fix compression test
---
src/test/regress/expected/compression.out | 2 ++
src/test/regress/expected/compression_1.out | 2 ++
src/test/regress/sql/compression.sql | 3 +++
3 files changed, 7 insertions(+)
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 3de2886..15af027 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -1,4 +1,5 @@
\set HIDE_TOAST_COMPRESSION false
+SET default_toast_compression = 'pglz';
-- test creating table with compression method
CREATE TABLE cmdata(f1 text COMPRESSION pglz);
CREATE INDEX idx ON cmdata(f1);
@@ -245,6 +246,7 @@ CREATE TABLE cmdata2 (f1 text);
--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
f1 | text | | | | extended | lz4 | |
+SET default_toast_compression = 'pglz';
-- test alter compression method
ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4;
INSERT INTO cmdata VALUES (repeat('123456789', 4004));
diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out
index 40aad81..8842997 100644
--- a/src/test/regress/expected/compression_1.out
+++ b/src/test/regress/expected/compression_1.out
@@ -1,4 +1,5 @@
\set HIDE_TOAST_COMPRESSION false
+SET default_toast_compression = 'pglz';
-- test creating table with compression method
CREATE TABLE cmdata(f1 text COMPRESSION pglz);
CREATE INDEX idx ON cmdata(f1);
@@ -241,6 +242,7 @@ CREATE TABLE cmdata2 (f1 text);
--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
f1 | text | | | | extended | pglz | |
+SET default_toast_compression = 'pglz';
-- test alter compression method
ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4;
ERROR: unsupported LZ4 compression method
diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql
index d97e26b..1280fa4 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -1,5 +1,7 @@
\set HIDE_TOAST_COMPRESSION false
+SET default_toast_compression = 'pglz';
+
-- test creating table with compression method
CREATE TABLE cmdata(f1 text COMPRESSION pglz);
CREATE INDEX idx ON cmdata(f1);
@@ -100,6 +102,7 @@ SET default_toast_compression = 'lz4';
DROP TABLE cmdata2;
CREATE TABLE cmdata2 (f1 text);
\d+ cmdata2
+SET default_toast_compression = 'pglz';
-- test alter compression method
ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4;
--
1.8.3.1
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
@ 2021-03-21 20:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-21 20:32 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Dilip Kumar <dilipbalaut@gmail.com> writes:
>> Yeah, we need to set the default_toast_compression in the beginning of
>> the test as attached.
> In the last patch, I did not adjust the compression_1.out so fixed
> that in the attached patch.
Pushed that; however, while testing that it works as expected,
I saw a new and far more concerning regression diff:
diff -U3 /home/postgres/pgsql/src/test/regress/expected/strings.out /home/postgres/pgsql/src/test/regress/results/strings.out
--- /home/postgres/pgsql/src/test/regress/expected/strings.out 2021-02-18 10:34:58.190304138 -0500
+++ /home/postgres/pgsql/src/test/regress/results/strings.out 2021-03-21 16:27:22.029402834 -0400
@@ -1443,10 +1443,10 @@
-- If start plus length is > string length, the result is truncated to
-- string length
SELECT substr(f1, 99995, 10) from toasttest;
- substr
---------
- 567890
- 567890
+ substr
+------------------------
+ 567890\x7F\x7F\x7F\x7F
+ 567890\x7F\x7F\x7F\x7F
567890
567890
(4 rows)
@@ -1520,10 +1520,10 @@
-- If start plus length is > string length, the result is truncated to
-- string length
SELECT substr(f1, 99995, 10) from toasttest;
- substr
---------
- 567890
- 567890
+ substr
+------------------------
+ 567890\177\177\177\177
+ 567890\177\177\177\177
567890
567890
(4 rows)
This seems somewhat repeatable (three identical failures in three
attempts). Not sure why I did not see it yesterday; but anyway,
there is something wrong with partial detoasting for LZ4.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 20:41 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Justin Pryzby @ 2021-03-21 20:41 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote:
> This seems somewhat repeatable (three identical failures in three
> attempts). Not sure why I did not see it yesterday; but anyway,
> there is something wrong with partial detoasting for LZ4.
With what version of LZ4 ?
--
Justin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
@ 2021-03-21 21:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-21 21:05 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote:
>> This seems somewhat repeatable (three identical failures in three
>> attempts). Not sure why I did not see it yesterday; but anyway,
>> there is something wrong with partial detoasting for LZ4.
> With what version of LZ4 ?
RHEL8's, which is
lz4-1.8.3-2.el8.x86_64
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-21 23:11 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
I wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
>> On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote:
>>> This seems somewhat repeatable (three identical failures in three
>>> attempts). Not sure why I did not see it yesterday; but anyway,
>>> there is something wrong with partial detoasting for LZ4.
>> With what version of LZ4 ?
> RHEL8's, which is
> lz4-1.8.3-2.el8.x86_64
I hate to be the bearer of bad news, but this suggests that
LZ4_decompress_safe_partial is seriously broken in 1.9.2
as well:
https://github.com/lz4/lz4/issues/783
Maybe we cannot rely on that function for a few more years yet.
Also, I don't really understand why this code:
/* slice decompression not supported prior to 1.8.3 */
if (LZ4_versionNumber() < 10803)
return lz4_decompress_datum(value);
It seems likely to me that we'd get a flat out build failure
from library versions lacking LZ4_decompress_safe_partial,
and thus that this run-time test is dead code and we should
better be using a configure probe if we intend to allow old
liblz4 versions. Though that might be moot.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 23:43 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Justin Pryzby @ 2021-03-21 23:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, Mar 21, 2021 at 07:11:50PM -0400, Tom Lane wrote:
> I wrote:
> > Justin Pryzby <pryzby@telsasoft.com> writes:
> >> On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote:
> >>> This seems somewhat repeatable (three identical failures in three
> >>> attempts). Not sure why I did not see it yesterday; but anyway,
> >>> there is something wrong with partial detoasting for LZ4.
>
> >> With what version of LZ4 ?
>
> > RHEL8's, which is
> > lz4-1.8.3-2.el8.x86_64
>
> I hate to be the bearer of bad news, but this suggests that
> LZ4_decompress_safe_partial is seriously broken in 1.9.2
> as well:
>
> https://github.com/lz4/lz4/issues/783
Ouch
> Maybe we cannot rely on that function for a few more years yet.
>
> Also, I don't really understand why this code:
>
> /* slice decompression not supported prior to 1.8.3 */
> if (LZ4_versionNumber() < 10803)
> return lz4_decompress_datum(value);
>
> It seems likely to me that we'd get a flat out build failure
> from library versions lacking LZ4_decompress_safe_partial,
> and thus that this run-time test is dead code and we should
> better be using a configure probe if we intend to allow old
> liblz4 versions. Though that might be moot.
The function existed before 1.8.3, but didn't handle slicing.
https://github.com/lz4/lz4/releases/tag/v1.8.3
|Finally, an existing function, LZ4_decompress_safe_partial(), has been enhanced to make it possible to decompress only the beginning of an LZ4 block, up to a specified number of bytes. Partial decoding can be useful to save CPU time and memory, when the objective is to extract a limited portion from a larger block.
Possibly we could allow v >= 1.9.3 || (ver >= 1.8.3 && ver < 1.9.2).
Or maybe not: the second half apparently worked "by accident", and we shouldn't
need to have intimate knowledge of someone else's patchlevel releases,
--
Justin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
@ 2021-03-21 23:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-21 23:52 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sun, Mar 21, 2021 at 07:11:50PM -0400, Tom Lane wrote:
>> I hate to be the bearer of bad news, but this suggests that
>> LZ4_decompress_safe_partial is seriously broken in 1.9.2
>> as well:
>> https://github.com/lz4/lz4/issues/783
> Ouch
Actually, after reading that closer, the problem only affects the
case where the compressed-data-length passed to the function is
a lie. So it shouldn't be a problem for our usage.
Also, after studying the documentation for LZ4_decompress_safe
and LZ4_decompress_safe_partial, I realized that liblz4 is also
counting on the *output* buffer size to not be a lie. So we
cannot pass it a number larger than the chunk's true decompressed
size. The attached patch resolves the issue I'm seeing.
regards, tom lane
Attachments:
fix-partial-lz4-decompression.patch (text/x-diff, 682B) download
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
index 00af1740cf..74e449992a 100644
--- a/src/backend/access/common/toast_compression.c
+++ b/src/backend/access/common/toast_compression.c
@@ -220,6 +220,10 @@ lz4_decompress_datum_slice(const struct varlena *value, int32 slicelength)
if (LZ4_versionNumber() < 10803)
return lz4_decompress_datum(value);
+ /* liblz4 assumes that slicelength is not an overestimate */
+ if (slicelength >= VARRAWSIZE_4B_C(value))
+ return lz4_decompress_datum(value);
+
/* allocate memory for the uncompressed data */
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
=
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-22 07:50 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Dilip Kumar @ 2021-03-22 07:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Mar 22, 2021 at 5:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Actually, after reading that closer, the problem only affects the
> case where the compressed-data-length passed to the function is
> a lie. So it shouldn't be a problem for our usage.
>
> Also, after studying the documentation for LZ4_decompress_safe
> and LZ4_decompress_safe_partial, I realized that liblz4 is also
> counting on the *output* buffer size to not be a lie. So we
> cannot pass it a number larger than the chunk's true decompressed
> size. The attached patch resolves the issue I'm seeing.
Okay, the fix makes sense. In fact, IMHO, in general also this fix
looks like an optimization, I mean when slicelength >=
VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
even in the case of pglz. So shall we put this check directly in
toast_decompress_datum_slice instead of handling it at the lz4 level?
Like this.
diff --git a/src/backend/access/common/detoast.c
b/src/backend/access/common/detoast.c
index bed50e8..099ac15 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -506,6 +506,10 @@ toast_decompress_datum_slice(struct varlena
*attr, int32 slicelength)
Assert(VARATT_IS_COMPRESSED(attr));
+ /* liblz4 assumes that slicelength is not an overestimate */
+ if (slicelength >= VARRAWSIZE_4B_C(attr))
+ return toast_decompress_datum(attr);
+
/*
* Fetch the compression method id stored in the compression header and
* decompress the data slice using the appropriate
decompression routine.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
@ 2021-03-22 14:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:27 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 15:29 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Tom Lane @ 2021-03-22 14:41 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Dilip Kumar <dilipbalaut@gmail.com> writes:
> On Mon, Mar 22, 2021 at 5:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Also, after studying the documentation for LZ4_decompress_safe
>> and LZ4_decompress_safe_partial, I realized that liblz4 is also
>> counting on the *output* buffer size to not be a lie. So we
>> cannot pass it a number larger than the chunk's true decompressed
>> size. The attached patch resolves the issue I'm seeing.
> Okay, the fix makes sense. In fact, IMHO, in general also this fix
> looks like an optimization, I mean when slicelength >=
> VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
> even in the case of pglz. So shall we put this check directly in
> toast_decompress_datum_slice instead of handling it at the lz4 level?
Yeah, I thought about that too, but do we want to assume that
VARRAWSIZE_4B_C is the correct way to get the decompressed size
for all compression methods?
(If so, I think it would be better style to have a less opaque macro
name for the purpose.)
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-22 15:27 ` Dilip Kumar <dilipbalaut@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Dilip Kumar @ 2021-03-22 15:27 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Mar 22, 2021 at 8:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Dilip Kumar <dilipbalaut@gmail.com> writes:
> > On Mon, Mar 22, 2021 at 5:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Also, after studying the documentation for LZ4_decompress_safe
> >> and LZ4_decompress_safe_partial, I realized that liblz4 is also
> >> counting on the *output* buffer size to not be a lie. So we
> >> cannot pass it a number larger than the chunk's true decompressed
> >> size. The attached patch resolves the issue I'm seeing.
>
> > Okay, the fix makes sense. In fact, IMHO, in general also this fix
> > looks like an optimization, I mean when slicelength >=
> > VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
> > even in the case of pglz. So shall we put this check directly in
> > toast_decompress_datum_slice instead of handling it at the lz4 level?
>
> Yeah, I thought about that too, but do we want to assume that
> VARRAWSIZE_4B_C is the correct way to get the decompressed size
> for all compression methods?
Yeah, VARRAWSIZE_4B_C is the macro getting the rawsize of the data
stored in the compressed varlena.
> (If so, I think it would be better style to have a less opaque macro
> name for the purpose.)
Okay, I have added another macro that is less opaque and came up with
this patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v1-0001-fix-slice-decompression.patch (application/octet-stream, 1.8K) download
From 343801cf8dfa79c3983cab813c5c48db3a390adb Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Mon, 22 Mar 2021 20:40:43 +0530
Subject: [PATCH v1] fix slice decompression
If the slicelength is more than the actual size of the data then no
point in trying to decompress the slice so directly decompress complete
data.
---
src/backend/access/common/detoast.c | 8 ++++++++
src/include/access/toast_internals.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index bed50e8..e6e57ad 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -506,6 +506,14 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
Assert(VARATT_IS_COMPRESSED(attr));
+ /*
+ * If the slicelength is more than the actual size of the data then no
+ * point in trying to decompress the slice so directly decompress complete
+ * data.
+ */
+ if (slicelength >= TOAST_COMPRESS_RAWSIZE(attr))
+ return toast_decompress_datum(attr);
+
/*
* Fetch the compression method id stored in the compression header and
* decompress the data slice using the appropriate decompression routine.
diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h
index b4d0684..e1e3521 100644
--- a/src/include/access/toast_internals.h
+++ b/src/include/access/toast_internals.h
@@ -31,6 +31,8 @@ typedef struct toast_compress_header
* Utilities for manipulation of header information for compressed
* toast entries.
*/
+#define TOAST_COMPRESS_RAWSIZE(ptr) \
+ (((toast_compress_header *) (ptr))->tcinfo & VARLENA_RAWSIZE_MASK)
#define TOAST_COMPRESS_METHOD(ptr) \
(((toast_compress_header *) (ptr))->tcinfo >> VARLENA_RAWSIZE_BITS)
#define TOAST_COMPRESS_SET_SIZE_AND_METHOD(ptr, len, cm_method) \
--
1.8.3.1
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-22 15:29 ` Robert Haas <robertmhaas@gmail.com>
2021-03-22 15:47 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 21+ messages in thread
From: Robert Haas @ 2021-03-22 15:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Mar 22, 2021 at 10:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Okay, the fix makes sense. In fact, IMHO, in general also this fix
> > looks like an optimization, I mean when slicelength >=
> > VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
> > even in the case of pglz. So shall we put this check directly in
> > toast_decompress_datum_slice instead of handling it at the lz4 level?
>
> Yeah, I thought about that too, but do we want to assume that
> VARRAWSIZE_4B_C is the correct way to get the decompressed size
> for all compression methods?
I think it's OK to assume this. If and when we add a third compression
method, it seems certain to just grab one of the two remaining bit
patterns. Now, things get a bit more complicated if and when we want
to add a fourth method, because at that point you've got to ask
yourself how comfortable you feel about stealing the last bit pattern
for your feature. But, if the solution to that problem were to decide
that whenever that last bit pattern is used, we will add an extra byte
(or word) after va_tcinfo indicating the real compression method, then
using VARRAWSIZE_4B_C here would still be correct. To imagine this
decision being wrong, you have to posit a world in which one of the
two remaining bit patterns for the high 2 bits cause the low 30 bits
to be interpreted as something other than the size, which I guess is
not totally impossible, but my first reaction is to think that such a
design would be (1) hard to make work and (2) unnecessarily painful.
> (If so, I think it would be better style to have a less opaque macro
> name for the purpose.)
Complaining about the name of one particular TOAST-related macro name
seems a bit like complaining about the greenhouse gasses emitted by
one particular car. They're pretty uniformly terrible. Does anyone
really know when to use VARATT_IS_1B_E or VARATT_IS_4B_U or any of
that cruft? Like, who decided that "is this varatt 1B E?" would be a
perfectly reasonable way of asking "is this varlena is TOAST
pointer?". While I'm complaining, it's hard to say enough bad things
about the fact that we have 12 consecutive completely obscure macro
definitions for which the only comments are (a) that they are
endian-dependent - which isn't even true for all of them - and (b)
that they are "considered internal." Apparently, they're SO internal
that they don't even need to be understandable to other developers.
Anyway, this particular macro name was chosen, it seems, for symmetry
with VARDATA_4B_C, but if you want to change it to something else, I'm
OK with that, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:29 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
@ 2021-03-22 15:47 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:59 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2021-03-22 15:47 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Mar 22, 2021 at 10:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, I thought about that too, but do we want to assume that
>> VARRAWSIZE_4B_C is the correct way to get the decompressed size
>> for all compression methods?
> I think it's OK to assume this.
OK, cool.
>> (If so, I think it would be better style to have a less opaque macro
>> name for the purpose.)
> Complaining about the name of one particular TOAST-related macro name
> seems a bit like complaining about the greenhouse gasses emitted by
> one particular car.
Maybe, but that's not a reason to make it worse. Anyway, my understanding
of that is that the really opaque names are *only* meant to be used in
this very stretch of postgres.h, ie they are just intermediate steps on
the way to the macros below them. As an example, the only use of
VARDATA_1B_E() is in VARDATA_EXTERNAL().
> Anyway, this particular macro name was chosen, it seems, for symmetry
> with VARDATA_4B_C, but if you want to change it to something else, I'm
> OK with that, too.
After looking at postgres.h for a bit, I'm thinking that what these
should have been symmetric with is the considerably-less-terrible
names used for the corresponding VARATT_EXTERNAL cases. Thus,
something like
s/VARRAWSIZE_4B_C/VARDATA_COMPRESSED_GET_RAWSIZE/
s/VARCOMPRESS_4B_C/VARDATA_COMPRESSED_GET_COMPRESSION/
Possibly the former names should survive and the latter become
wrappers around them, not sure. But we shouldn't be using the "4B"
terminology anyplace except this part of postgres.h.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:29 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
2021-03-22 15:47 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-22 15:59 ` Robert Haas <robertmhaas@gmail.com>
2021-03-22 16:04 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 21+ messages in thread
From: Robert Haas @ 2021-03-22 15:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Mar 22, 2021 at 11:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Anyway, this particular macro name was chosen, it seems, for symmetry
> > with VARDATA_4B_C, but if you want to change it to something else, I'm
> > OK with that, too.
>
> After looking at postgres.h for a bit, I'm thinking that what these
> should have been symmetric with is the considerably-less-terrible
> names used for the corresponding VARATT_EXTERNAL cases. Thus,
> something like
>
> s/VARRAWSIZE_4B_C/VARDATA_COMPRESSED_GET_RAWSIZE/
> s/VARCOMPRESS_4B_C/VARDATA_COMPRESSED_GET_COMPRESSION/
Works for me.
> Possibly the former names should survive and the latter become
> wrappers around them, not sure. But we shouldn't be using the "4B"
> terminology anyplace except this part of postgres.h.
I would argue that it shouldn't be used any place at all, and that we
ought to go the other direction and get rid of the existing macros -
e.g. change #define VARATT_IS_1B_E to #define VARATT_IS_EXTERNAL
instead of defining the latter as a no-value-added wrapper around the
former. Maybe at one time somebody thought that the test for
VARATT_IS_EXTERNAL might someday have more cases than just
VARATT_IS_1B_E, but that's not looking like a good bet in 2021.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:29 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
2021-03-22 15:47 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:59 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Robert Haas <robertmhaas@gmail.com>
@ 2021-03-22 16:04 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 21+ messages in thread
From: Tom Lane @ 2021-03-22 16:04 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Mar 22, 2021 at 11:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Possibly the former names should survive and the latter become
>> wrappers around them, not sure. But we shouldn't be using the "4B"
>> terminology anyplace except this part of postgres.h.
> I would argue that it shouldn't be used any place at all, and that we
> ought to go the other direction and get rid of the existing macros -
> e.g. change #define VARATT_IS_1B_E to #define VARATT_IS_EXTERNAL
> instead of defining the latter as a no-value-added wrapper around the
> former. Maybe at one time somebody thought that the test for
> VARATT_IS_EXTERNAL might someday have more cases than just
> VARATT_IS_1B_E, but that's not looking like a good bet in 2021.
Maybe. I think the original idea was exactly what the comment says,
to have a layer of macros that'd deal with endianness issues and no more.
That still seems like a reasonable plan to me, though perhaps it wasn't
executed very well.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [HACKERS] Custom compression methods (mac+lz4.h)
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Re: [HACKERS] Custom compression methods (mac+lz4.h) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-03-21 07:37 ` Dilip Kumar <dilipbalaut@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Dilip Kumar @ 2021-03-21 07:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Alexander Korotkov <a.korotkov@postgrespro.ru>; David Steele <david@pgmasters.net>; Ildus Kurbangaliev <i.kurbangaliev@gmail.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Andrey Borodin <x4mmm@yandex-team.ru>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, Mar 21, 2021 at 7:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Also, I see some diffs in the
> indirect_toast test, which seems perhaps worthy of investigation.
> (The diffs look to be just row ordering, but why?)
I have investigated that, actually in the below insert, after
compression the data size of (repeat('1234567890',50000)) is 1980
bytes with the lz4 whereas with pglz it is 5737 bytes. So with lz4,
the compressed data are stored inline whereas with pglz those are
getting externalized. Due to this for one of the update statements
followed by an insert, there was no space on the first page as data
are stored inline so the new tuple is stored on the next page and that
is what affecting the order. I hope this makes sense.
INSERT INTO indtoasttest(descr, f1, f2) VALUES('one-toasted,one-null',
NULL, repeat('1234567890',50000));
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
end of thread, other threads:[~2021-03-22 16:04 UTC | newest]
Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-20 21:23 Re: [HACKERS] Custom compression methods (mac+lz4.h) Justin Pryzby <pryzby@telsasoft.com>
2021-03-20 21:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-20 23:19 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 00:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 01:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 03:40 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 05:22 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-21 20:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 20:41 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 21:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 23:43 ` Justin Pryzby <pryzby@telsasoft.com>
2021-03-21 23:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 07:50 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 14:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:27 ` Dilip Kumar <dilipbalaut@gmail.com>
2021-03-22 15:29 ` Robert Haas <robertmhaas@gmail.com>
2021-03-22 15:47 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-22 15:59 ` Robert Haas <robertmhaas@gmail.com>
2021-03-22 16:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-21 07:37 ` Dilip Kumar <dilipbalaut@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox