From: | Nikhil Kumar Veldanda <veldanda(dot)nikhilkumar17(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: ZStandard (with dictionaries) compression support for TOAST compression |
Date: | 2025-07-16 05:37:02 |
Message-ID: | CAFAfj_GPC6ki9qgyyoZPPzsepPr7du_8uRRCnmKUaONeGYxWpQ@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Michael,
On Tue, Jul 15, 2025 at 9:44 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> I have no idea yet about the fate of the other TOAST patches I have
> proposed for this commit fest, but let's move on with this part of the
> refactoring by splitting the TOAST regression tests for LZ4 and pglz,
> with the new pg_compression_available() that would reduce the diffs
> with the alternate outputs.
>
> This has required a bit more work than I suspected. Based on my
> notes, first for pg_compression_available():
> - Code moved to misc.c, with comments related to TOAST removed.
> - Addition of gzip as an acceptable value.
> - Error if the compression method is unknown.
> - Some regression tests.
> - Documentation should list the functions alphabetically.
>
> Then for the refactoring of the tests, a few notes:
> - There is no need for cmdata1 in compression.sql, using the same
> compression method as cmdata, aka pglz. So we can trim down the
> tests.
> - In compression.sql, we can remove cmmove2, cmmove3 and cmdata2 which
> have a compression method of pglz, and that we want to check where the
> origin has LZ4 data. These should be only in compression_lz4.sql,
> perhaps also in the zstd portion if needed later for your patch.
> - The error cases with I_Do_Not_Exist_Compression at the bottom of
> compression.sql can be kept, we don't need them in
> compression_lz4.sql.
> - It would be tempting to keep the test for LIKE INCLUDING COMPRESSION
> in compression.sql, but we cannot do that as there is a dependency
> with default_toast_compression so we want the GUC at pglz but the
> table we are copying the data from at LZ4.
> compression.sql, there is no need for it to depend on LZ4.
> - The tests related to cmdata2 depend on LZ4 TOAST, which were a bit
> duplicated.
> - "test column type update varlena/non-varlena" was duplicated. Same
> for "changing column storage should not impact the compression
> method".
> - The materialized view test in compression.sql depends on LZ4, can be
> moved to compression_lz4.sql.
> - The test with partitions and compression methods expects multiple
> compression methods, can be moved to compression_lz4.sql
> - "test alter compression method" expects two compression methods, can
> be moved to compression_lz4.sql.
> - The tests with SET default_toast_compression report a hint with the
> list of values supported. This is not portable because the list of
> values depends on what the build supports. We should use a trick
> based on "\set VERBOSITY terse", removing the HINT to reduce the
> noise.
> - The tables specific to pglz and lz4 data are both still required in
> compression_lz4.sql, for one test with inheritance. I have renamed
> both to cmdata_pglz and cmdata_lz4, for clarity.
>
> At the end, the gain in diffs is here per the following numbers in
> the attached 0002 as we remove the alternal output of compression.sql
> when lz4 is disabled:
> 7 files changed, 319 insertions(+), 724 deletions(-)
>
> Attached are two patches for all that:
> - 0001: Introduction of the new function pg_compression_available().
> - 0002: Refactoring of the TOAST compression tests.
>
> With this infrastructure in place, the addition of a new TOAST
> compression method becomes easier for the test part: no more
> cross-build specific diffs.
>
> Thought, comments or objections?
>
Thanks for driving this forward—both patches look good to me.
0001 – pg_compression_available()
pg_compression_available() in misc.c feels sensible;
0002 – test-suite split
The new compression.sql / compression_lz4.sql split makes the diffs
much easier to reason about.
> Michael
--
Nikhil Veldanda
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2025-07-16 05:44:44 | Re: Logical Replication of sequences |
Previous Message | Michael Paquier | 2025-07-16 04:44:15 | Re: ZStandard (with dictionaries) compression support for TOAST compression |