Re: [HACKERS] Custom compression methods

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, David Steele <david(at)pgmasters(dot)net>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)gmail(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [HACKERS] Custom compression methods
Date: 2021-03-08 10:02:39
Message-ID: CAFiTN-tjZrn1OrykBUvzf4_GU7HUPBjSbdwM8_SVnD-9gMWHdA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Mar 7, 2021 at 1:27 AM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>
> On Sat, Mar 06, 2021 at 08:59:16PM +0530, Dilip Kumar wrote:
> > - Alter table set compression, will not rewrite the old data, so only
> > the new tuple will be compressed with the new compression method.
> > - No preserve.
>
> In this patch, SET default_toast_compression=lz4 "works" even if without-lz4,
> but then CREATE TABLE fails. You should either allow table creation (as
> above), or check in check_default_toast_compression() if lz4 is enabled.
> Its comment about "catalog access" is incorrect now.

As of now I have made GUC behavior similar to the CREATE TABLE, in
both case it will throw an error if it is not compiled with lz4
method.

>
> + if (strcmp(def->compression, newdef->compression))
> != 0
>
> + * NULL for non varlena type or the uncompressed data.
> remove "the"
>
> + * InvalidOid for the plain/external storage otherwise default
> remove "the"
>
> + behavior is to exclude compression methods, resulting in the columns
> remove "the"
>
>
> + attcompression and attstorage for the respective index attribute if ... the respective input values are
> say "and/or attstorage"
>
> + If this variable is set to <literal>true</literal>, column's [...]
> I wrote this, but I guess it should say: columns'
>
> I think you could also say either of these:
> .. column compression method details are not displayed.
> .. details of column compression are not displayed.

I have fixed the above comments, and also some other minor fixup.

So now only pending point is, how do we handle the upgrade when you
are upgrading from --with-lz4 to --without-lz4 binary and a couple of
options discussed here are
a) Should we allow table creation with lz4 even if it is compiled
--without-lz4? In case of xml we always allow table creation even if
it is compiled --wthout-libxml
b) Instead of allowing this always, only allow during binary upgrade.

With this we will be able to make binary upgrades successful.
However, if users have to access the data compiled with the lz4 then
then need to enable the lz4 support. But I think that is true with
any case for example with a hot standby setup if the user configures
the standby without lz4 then also for accessing the lz4 compressed
data user needs to enable the lz4 support.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v32-0001-Built-in-compression-method.patch application/x-patch 91.3 KB
v32-0002-Add-default_toast_compression-GUC.patch application/x-patch 7.8 KB
v32-0004-default-to-with-lz4.patch application/x-patch 1.7 KB
v32-0003-Alter-table-set-compression.patch application/x-patch 20.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-03-08 10:16:16 Re: Boundary value check in lazy_tid_reaped()
Previous Message Peter Eisentraut 2021-03-08 09:57:56 Re: authtype parameter in libpq