public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 4/8] Propagate changes to indisclustered to child/parents
15+ messages / 10 participants
[nested] [flat]
* [PATCH v9 4/8] Propagate changes to indisclustered to child/parents
@ 2020-10-07 03:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw)
---
src/backend/commands/cluster.c | 110 ++++++++++++++++----------
src/backend/commands/indexcmds.c | 2 +
src/test/regress/expected/cluster.out | 46 +++++++++++
src/test/regress/sql/cluster.sql | 11 +++
4 files changed, 126 insertions(+), 43 deletions(-)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 7d8d1d8a69..dd8014c206 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
+static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index);
static List *get_tables_to_cluster(MemoryContext cluster_context);
static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
Oid indexOid);
@@ -513,66 +514,89 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
index_close(OldIndex, NoLock);
}
+/*
+ * Helper for mark_index_clustered
+ * Mark a single index as clustered or not.
+ * pg_index is passed by caller to avoid repeatedly re-opening it.
+ */
+static void
+set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index)
+{
+ HeapTuple indexTuple;
+ Form_pg_index indexForm;
+
+ indexTuple = SearchSysCacheCopy1(INDEXRELID,
+ ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(indexTuple))
+ elog(ERROR, "cache lookup failed for index %u", indexOid);
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+ /* this was checked earlier, but let's be real sure */
+ if (isclustered && !indexForm->indisvalid)
+ elog(ERROR, "cannot cluster on invalid index %u", indexOid);
+
+ indexForm->indisclustered = isclustered;
+ CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
+ heap_freetuple(indexTuple);
+}
+
/*
* mark_index_clustered: mark the specified index as the one clustered on
*
- * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
+ * With indexOid == InvalidOid, mark all indexes of rel not-clustered.
+ * Otherwise, mark children of the clustered index as clustered, and parents of
+ * other indexes as unclustered.
+ * We wish to maintain the following properties:
+ * 1) Only one index on a relation can be marked clustered at once
+ * 2) If a partitioned index is clustered, then all its children must be
+ * clustered.
*/
void
mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
{
- HeapTuple indexTuple;
- Form_pg_index indexForm;
- Relation pg_index;
- ListCell *index;
-
- /*
- * If the index is already marked clustered, no need to do anything.
- */
- if (OidIsValid(indexOid))
- {
- if (get_index_isclustered(indexOid))
- return;
- }
+ ListCell *lc, *lc2;
+ List *indexes;
+ Relation pg_index = table_open(IndexRelationId, RowExclusiveLock);
+ List *inh = find_all_inheritors(RelationGetRelid(rel),
+ ShareRowExclusiveLock, NULL);
/*
* Check each index of the relation and set/clear the bit as needed.
+ * Iterate over the relation's children rather than the index's children
+ * since we need to unset cluster for indexes on intermediate children,
+ * too.
*/
- pg_index = table_open(IndexRelationId, RowExclusiveLock);
-
- foreach(index, RelationGetIndexList(rel))
+ foreach(lc, inh)
{
- Oid thisIndexOid = lfirst_oid(index);
-
- indexTuple = SearchSysCacheCopy1(INDEXRELID,
- ObjectIdGetDatum(thisIndexOid));
- if (!HeapTupleIsValid(indexTuple))
- elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
- indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+ Oid inhrelid = lfirst_oid(lc);
+ Relation thisrel = table_open(inhrelid, ShareRowExclusiveLock);
- /*
- * Unset the bit if set. We know it's wrong because we checked this
- * earlier.
- */
- if (indexForm->indisclustered)
+ indexes = RelationGetIndexList(thisrel);
+ foreach (lc2, indexes)
{
- indexForm->indisclustered = false;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
- else if (thisIndexOid == indexOid)
- {
- /* this was checked earlier, but let's be real sure */
- if (!indexForm->indisvalid)
- elog(ERROR, "cannot cluster on invalid index %u", indexOid);
- indexForm->indisclustered = true;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
+ bool isclustered;
+ Oid thisIndexOid = lfirst_oid(lc2);
+ List *parentoids = get_rel_relispartition(thisIndexOid) ?
+ get_partition_ancestors(thisIndexOid) : NIL;
- InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
- InvalidOid, is_internal);
+ /*
+ * A child of the clustered index must be set clustered;
+ * indexes which are not children of the clustered index are
+ * set unclustered
+ */
+ isclustered = (thisIndexOid == indexOid) ||
+ list_member_oid(parentoids, indexOid);
+ Assert(OidIsValid(indexOid) || !isclustered);
+ set_indisclustered(thisIndexOid, isclustered, pg_index);
+
+ InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
+ InvalidOid, is_internal);
+ }
- heap_freetuple(indexTuple);
+ list_free(indexes);
+ table_close(thisrel, ShareRowExclusiveLock);
}
+ list_free(inh);
table_close(pg_index, RowExclusiveLock);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8bc652ecd3..5f8fc2ea16 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -25,6 +25,7 @@
#include "catalog/catalog.h"
#include "catalog/index.h"
#include "catalog/indexing.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_inherits.h"
@@ -32,6 +33,7 @@
#include "catalog/pg_opfamily.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
+#include "commands/cluster.h"
#include "commands/comment.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index c74cfa88cc..1d436dfaae 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -495,6 +495,52 @@ Indexes:
"clstrpart_idx" btree (a) CLUSTER
Number of partitions: 3 (Use \d+ to list them.)
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a)
+
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
+ Partitioned table "public.clstrpart1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart FOR VALUES FROM (1) TO (10)
+Partition key: RANGE (a)
+Indexes:
+ "clstrpart1_a_idx" btree (a)
+ "clstrpart1_idx_2" btree (a) CLUSTER
+Number of partitions: 2 (Use \d+ to list them.)
+
-- Test CLUSTER with external tuplesorting
create table clstr_4 as select * from tenk1;
create index cluster_sort on clstr_4 (hundred, thousand, tenthous);
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 9bcc77695c..0ded2be1ca 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio
CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
\d clstrpart
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
-- Test CLUSTER with external tuplesorting
--
2.17.0
--fmvA4kSBHQVZhkR6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0005-Invalidate-parent-indexes.patch"
^ permalink raw reply [nested|flat] 15+ messages in thread
* change default default_toast_compression to lz4?
@ 2025-11-21 11:11 Peter Eisentraut <[email protected]>
2025-11-21 11:33 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-11-21 16:10 ` Re: change default default_toast_compression to lz4? Andres Freund <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
0 siblings, 3 replies; 15+ messages in thread
From: Peter Eisentraut @ 2025-11-21 11:11 UTC (permalink / raw)
To: pgsql-hackers
How about changing the default of default_toast_compression to lz4?
I have seen cases where servers had performance problems and were
CPU-bound because of pglz TOAST compression, and changing it to lz4
relieved it significantly. I suspect many users are leaving easy
performance improvements on the table by not using this option.
The feature was introduced in PostgreSQL 14, so it should be well
stabilized now.
I suppose one issue is that lz4 support is not compiled-in by default,
but in practice most users will have it. The default could be lz4 if
lz4 support is built, otherwise pglz. This would be similar to other
parameters where the default is the best one depending on the build
configuration.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
@ 2025-11-21 11:33 ` Aleksander Alekseev <[email protected]>
2025-11-21 12:27 ` Re: change default default_toast_compression to lz4? Daniel Gustafsson <[email protected]>
2 siblings, 1 reply; 15+ messages in thread
From: Aleksander Alekseev @ 2025-11-21 11:33 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Peter Eisentraut <[email protected]>
Hi Peter,
> How about changing the default of default_toast_compression to lz4?
To me it sounds like a great idea.
> I have seen cases where servers had performance problems and were
> CPU-bound because of pglz TOAST compression, and changing it to lz4
> relieved it significantly. I suspect many users are leaving easy
> performance improvements on the table by not using this option.
>
> The feature was introduced in PostgreSQL 14, so it should be well
> stabilized now.
>
> I suppose one issue is that lz4 support is not compiled-in by default,
> but in practice most users will have it. The default could be lz4 if
> lz4 support is built, otherwise pglz. This would be similar to other
> parameters where the default is the best one depending on the build
> configuration.
Are there good reasons why we can't simply make lz4 a required
dependency? In the worst case we could simply copy its implementation,
the license permits.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-21 11:33 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
@ 2025-11-21 12:27 ` Daniel Gustafsson <[email protected]>
2025-11-22 13:11 ` Re: change default default_toast_compression to lz4? Álvaro Herrera <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Gustafsson @ 2025-11-21 12:27 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Peter Eisentraut <[email protected]>
> On 21 Nov 2025, at 12:33, Aleksander Alekseev <[email protected]> wrote:
>> The default could be lz4 if
>> lz4 support is built, otherwise pglz. This would be similar to other
>> parameters where the default is the best one depending on the build
>> configuration.
+1
> Are there good reasons why we can't simply make lz4 a required
> dependency? In the worst case we could simply copy its implementation,
> the license permits.
I think we should, as much as we can, avoid vendoring code, especially
something like lz4 which can be expected to be available nearly everywhere.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-21 11:33 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-11-21 12:27 ` Re: change default default_toast_compression to lz4? Daniel Gustafsson <[email protected]>
@ 2025-11-22 13:11 ` Álvaro Herrera <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Álvaro Herrera @ 2025-11-22 13:11 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Peter Eisentraut <[email protected]>
On 2025-Nov-21, Daniel Gustafsson wrote:
> > On 21 Nov 2025, at 12:33, Aleksander Alekseev <[email protected]> wrote:
>
> > Are there good reasons why we can't simply make lz4 a required
> > dependency? In the worst case we could simply copy its implementation,
> > the license permits.
>
> I think we should, as much as we can, avoid vendoring code, especially
> something like lz4 which can be expected to be available nearly everywhere.
Yeah. There's the security aspect: if lz4 is found to have a security
bug, we would be obliged to issue an advisory and matching release.
It's best if the library code is kept separate, so their own security
advisory is enough.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
@ 2025-11-21 16:10 ` Andres Freund <[email protected]>
2025-11-21 16:16 ` Re: change default default_toast_compression to lz4? Tom Lane <[email protected]>
2 siblings, 1 reply; 15+ messages in thread
From: Andres Freund @ 2025-11-21 16:10 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
Hi,
On 2025-11-21 12:11:38 +0100, Peter Eisentraut wrote:
> How about changing the default of default_toast_compression to lz4?
>
> I have seen cases where servers had performance problems and were CPU-bound
> because of pglz TOAST compression, and changing it to lz4 relieved it
> significantly. I suspect many users are leaving easy performance
> improvements on the table by not using this option.
+1 - I also have seen *and* hit this numerous times.
IIRC it makes the tests runs a tad bit faster too.
> I suppose one issue is that lz4 support is not compiled-in by default, but
> in practice most users will have it. The default could be lz4 if lz4
> support is built, otherwise pglz. This would be similar to other parameters
> where the default is the best one depending on the build configuration.
I think we should mark lz4 as a default-required dependency if we change the
default. That way one needs to explicitly opt into a build that won't be
compatible with existing data directories created (due to pre-existing lz4 .
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-21 16:10 ` Re: change default default_toast_compression to lz4? Andres Freund <[email protected]>
@ 2025-11-21 16:16 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Tom Lane @ 2025-11-21 16:16 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Andres Freund <[email protected]> writes:
> On 2025-11-21 12:11:38 +0100, Peter Eisentraut wrote:
>> I suppose one issue is that lz4 support is not compiled-in by default, but
>> in practice most users will have it. The default could be lz4 if lz4
>> support is built, otherwise pglz. This would be similar to other parameters
>> where the default is the best one depending on the build configuration.
> I think we should mark lz4 as a default-required dependency if we change the
> default. That way one needs to explicitly opt into a build that won't be
> compatible with existing data directories created (due to pre-existing lz4 .
Agreed --- we should make it act like icu or readline, you have to
opt out. (I'm not voting one way or the other on changing the
default, but if we do we should do it that way.)
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
@ 2025-11-26 04:35 ` Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2025-11-26 04:35 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Fri, Nov 21, 2025 at 12:11:38PM +0100, Peter Eisentraut wrote:
> I suppose one issue is that lz4 support is not compiled-in by default, but
> in practice most users will have it. The default could be lz4 if lz4
> support is built, otherwise pglz. This would be similar to other parameters
> where the default is the best one depending on the build configuration.
+1. That makes sense here to flip the default depending on what the
code is built with, with lz4 if available, pglz otherwise.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
@ 2025-11-26 18:59 ` Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Euler Taveira @ 2025-11-26 18:59 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Wed, Nov 26, 2025, at 1:35 AM, Michael Paquier wrote:
> On Fri, Nov 21, 2025 at 12:11:38PM +0100, Peter Eisentraut wrote:
>> I suppose one issue is that lz4 support is not compiled-in by default, but
>> in practice most users will have it. The default could be lz4 if lz4
>> support is built, otherwise pglz. This would be similar to other parameters
>> where the default is the best one depending on the build configuration.
>
> +1. That makes sense here to flip the default depending on what the
> code is built with, with lz4 if available, pglz otherwise.
>
Since we have an agreement that $SUBJECT is ok, I wrote a patch for it. It
selects the compression method based on USE_LZ4. It also adjusts the
postgresql.conf if required.
--
Euler Taveira
EDB https://www.enterprisedb.com/
Attachments:
[text/x-patch] v1-0001-Change-default_toast_compression-to-lz4.patch (3.7K, ../../[email protected]/2-v1-0001-Change-default_toast_compression-to-lz4.patch)
download | inline diff:
From 89084e17df13823077bad2f6ad4c1cb0e095ccc4 Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
Date: Wed, 26 Nov 2025 12:40:43 -0300
Subject: [PATCH v1] Change default_toast_compression to lz4
The default value for default_toast_compression was pglz. The main
reason is that this option is always available. However, it is known
that pglz uses more CPU than lz4. The default value will be lz4 if lz4
support is built, otherwise, pglz.
---
doc/src/sgml/config.sgml | 3 ++-
src/backend/access/common/toast_compression.c | 2 +-
src/backend/utils/misc/guc_parameters.dat | 2 +-
src/bin/initdb/initdb.c | 5 +++++
src/include/access/toast_compression.h | 9 +++++++++
5 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 737b90736bf..c063f013e2f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9910,7 +9910,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The supported compression methods are <literal>pglz</literal> and
(if <productname>PostgreSQL</productname> was compiled with
<option>--with-lz4</option>) <literal>lz4</literal>.
- The default is <literal>pglz</literal>.
+ The default is <literal>lz4</literal> (if available); otherwise,
+ <literal>pglz</literal>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
index 926f1e4008a..4bb29665eab 100644
--- a/src/backend/access/common/toast_compression.c
+++ b/src/backend/access/common/toast_compression.c
@@ -23,7 +23,7 @@
#include "varatt.h"
/* GUC */
-int default_toast_compression = TOAST_PGLZ_COMPRESSION;
+int default_toast_compression = DEFAULT_TOAST_COMPRESSION;
#define NO_COMPRESSION_SUPPORT(method) \
ereport(ERROR, \
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3b9d8349078..cbe9bf055b3 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -735,7 +735,7 @@
{ name => 'default_toast_compression', type => 'enum', context => 'PGC_USERSET', group => 'CLIENT_CONN_STATEMENT',
short_desc => 'Sets the default compression method for compressible values.',
variable => 'default_toast_compression',
- boot_val => 'TOAST_PGLZ_COMPRESSION',
+ boot_val => 'DEFAULT_TOAST_COMPRESSION',
options => 'default_toast_compression_options',
},
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 92fe2f531f7..92b120d8ab8 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1424,6 +1424,11 @@ setup_config(void)
"0640", false);
}
+#if USE_LZ4
+ conflines = replace_guc_value(conflines, "default_toast_compression",
+ "lz4", true);
+#endif
+
/*
* Now replace anything that's overridden via -c switches.
*/
diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h
index 13c4612ceed..7526ea50c5a 100644
--- a/src/include/access/toast_compression.h
+++ b/src/include/access/toast_compression.h
@@ -52,6 +52,15 @@ typedef enum ToastCompressionId
#define CompressionMethodIsValid(cm) ((cm) != InvalidCompressionMethod)
+/*
+ * Choose an appropriate default toast compression method. If lz4 is
+ * compiled-in, use it, otherwise, use pglz.
+ */
+#ifdef USE_LZ4
+#define DEFAULT_TOAST_COMPRESSION TOAST_LZ4_COMPRESSION
+#else
+#define DEFAULT_TOAST_COMPRESSION TOAST_PGLZ_COMPRESSION
+#endif
/* pglz compression/decompression routines */
extern struct varlena *pglz_compress_datum(const struct varlena *value);
--
2.39.5
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
@ 2025-11-27 09:44 ` Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Aleksander Alekseev @ 2025-11-27 09:44 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Euler Taveira <[email protected]>; Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>
Hi Euler,
> Since we have an agreement that $SUBJECT is ok, I wrote a patch for it. It
> selects the compression method based on USE_LZ4. It also adjusts the
> postgresql.conf if required.
Many thanks for working on this. Unfortunately the patch is
incomplete. I think we also agreed that lz4 should be opt-out.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
@ 2025-12-04 20:11 ` Euler Taveira <[email protected]>
2025-12-05 02:23 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Euler Taveira @ 2025-12-04 20:11 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; pgsql-hackers; +Cc: Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>
On Thu, Nov 27, 2025, at 6:44 AM, Aleksander Alekseev wrote:
> Many thanks for working on this. Unfortunately the patch is
> incomplete. I think we also agreed that lz4 should be opt-out.
>
Aleksander, I missed this point.
Here it is v2. It enforces the lz4 dependency. It works the same as other
dependencies (readline, icu, zlib); error if the dependency could not be found.
Regarding meson, I'm confused. If any of the referred dependencies (icu,
readline, zlib) is not found, there is no hard error. Instead, the feature is
disabled. I searched for a discussion about this decision but couldn't find.
For this patch, I decided to use the same pattern (no error) but I added a
warning message (similar to zlib).
$ meson setup build --prefix=/tmp/pg | grep -i warning
meson.build:1100: WARNING: did not find lz4
meson.build:1675: WARNING: did not find zlib
The other alternative is to always 'enabled' lz4 in meson_options.txt. This
requires you to explicitly enable/disable lz4 if you are using -Dauto_features
option.
Should it report an error (like autoconf) instead of silently disable the
feature that is a requirement? If yes, then we should add error messages for
these 3 dependencies.
$ ./configure --prefix=/tmp/pg
.
.
checking whether to build with ICU support... yes
checking for icu-uc icu-i18n... no
configure: error: Package requirements (icu-uc icu-i18n) were not met:
Package 'icu-uc', required by 'virtual:world', not found
Package 'icu-i18n', required by 'virtual:world', not found
Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.
Alternatively, you may set the environment variables ICU_CFLAGS
and ICU_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
$ git clean -dfxq
$ meson setup build --prefix=/tmp/pg
.
.
Run-time dependency icu-uc found: NO (tried pkgconfig and cmake)
Run-time dependency icu found: NO (tried cmake)
.
.
Run-time dependency readline found: NO (tried pkgconfig and cmake)
Library readline found: NO
.
.
External libraries
bonjour : NO
bsd_auth : NO
docs : NO
docs_pdf : NO
gss : NO
icu : NO
ldap : NO
libcurl : NO
libnuma : NO
liburing : NO
libxml : NO
libxslt : NO
llvm : NO
lz4 : NO
nls : YES
openssl : NO
pam : NO
plperl : NO 5.40.1
plpython : NO
pltcl : NO
readline : NO
selinux : NO
systemd : NO
uuid : NO
zlib : NO
zstd : NO
User defined options
prefix : /tmp/pg
Found ninja-1.12.1 at /usr/bin/ninja
--
Euler Taveira
EDB https://www.enterprisedb.com/
Attachments:
[text/x-patch] v2-0001-Change-default_toast_compression-to-lz4.patch (5.3K, ../../[email protected]/2-v2-0001-Change-default_toast_compression-to-lz4.patch)
download | inline diff:
From d700609b2c194e00c7ded6ed9a16dd60427ff846 Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
Date: Wed, 26 Nov 2025 12:40:43 -0300
Subject: [PATCH v2] Change default_toast_compression to lz4
The default value for default_toast_compression was pglz. The main
reason is that this option is always available. However, it is known
that pglz uses more CPU than lz4. The default value will be lz4 if lz4
support is built, otherwise, pglz.
---
configure.ac | 2 +-
doc/src/sgml/config.sgml | 3 ++-
doc/src/sgml/installation.sgml | 12 ++++++++++++
meson.build | 2 ++
src/backend/access/common/toast_compression.c | 2 +-
src/backend/utils/misc/guc_parameters.dat | 2 +-
src/bin/initdb/initdb.c | 5 +++++
src/include/access/toast_compression.h | 9 +++++++++
8 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index c2413720a18..0a4598e511d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1133,7 +1133,7 @@ AC_SUBST(with_zlib)
# LZ4
#
AC_MSG_CHECKING([whether to build with LZ4 support])
-PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support],
+PGAC_ARG_BOOL(with, lz4, yes, [build without LZ4 support],
[AC_DEFINE([USE_LZ4], 1, [Define to 1 to build with LZ4 support. (--with-lz4)])])
AC_MSG_RESULT([$with_lz4])
AC_SUBST(with_lz4)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 405c9689bd0..73eb33e2d99 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9910,7 +9910,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The supported compression methods are <literal>pglz</literal> and
(if <productname>PostgreSQL</productname> was compiled with
<option>--with-lz4</option>) <literal>lz4</literal>.
- The default is <literal>pglz</literal>.
+ The default is <literal>lz4</literal> (if available); otherwise,
+ <literal>pglz</literal>.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index fe8d73e1f8c..f8b43a8cb7b 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1315,6 +1315,18 @@ build-postgresql:
</listitem>
</varlistentry>
+ <varlistentry id="configure-option-without-lz4">
+ <term><option>--without-lz4</option></term>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary>lz4</primary>
+ </indexterm>
+ Prevents use of the <application>LZ4</application> library.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect3>
diff --git a/meson.build b/meson.build
index 6e7ddd74683..1da53bf0138 100644
--- a/meson.build
+++ b/meson.build
@@ -1096,6 +1096,8 @@ if not lz4opt.disabled()
if lz4.found()
cdata.set('USE_LZ4', 1)
cdata.set('HAVE_LIBLZ4', 1)
+ else
+ warning('did not find lz4')
endif
else
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
index 926f1e4008a..4bb29665eab 100644
--- a/src/backend/access/common/toast_compression.c
+++ b/src/backend/access/common/toast_compression.c
@@ -23,7 +23,7 @@
#include "varatt.h"
/* GUC */
-int default_toast_compression = TOAST_PGLZ_COMPRESSION;
+int default_toast_compression = DEFAULT_TOAST_COMPRESSION;
#define NO_COMPRESSION_SUPPORT(method) \
ereport(ERROR, \
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3b9d8349078..cbe9bf055b3 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -735,7 +735,7 @@
{ name => 'default_toast_compression', type => 'enum', context => 'PGC_USERSET', group => 'CLIENT_CONN_STATEMENT',
short_desc => 'Sets the default compression method for compressible values.',
variable => 'default_toast_compression',
- boot_val => 'TOAST_PGLZ_COMPRESSION',
+ boot_val => 'DEFAULT_TOAST_COMPRESSION',
options => 'default_toast_compression_options',
},
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 92fe2f531f7..92b120d8ab8 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1424,6 +1424,11 @@ setup_config(void)
"0640", false);
}
+#if USE_LZ4
+ conflines = replace_guc_value(conflines, "default_toast_compression",
+ "lz4", true);
+#endif
+
/*
* Now replace anything that's overridden via -c switches.
*/
diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h
index 13c4612ceed..7526ea50c5a 100644
--- a/src/include/access/toast_compression.h
+++ b/src/include/access/toast_compression.h
@@ -52,6 +52,15 @@ typedef enum ToastCompressionId
#define CompressionMethodIsValid(cm) ((cm) != InvalidCompressionMethod)
+/*
+ * Choose an appropriate default toast compression method. If lz4 is
+ * compiled-in, use it, otherwise, use pglz.
+ */
+#ifdef USE_LZ4
+#define DEFAULT_TOAST_COMPRESSION TOAST_LZ4_COMPRESSION
+#else
+#define DEFAULT_TOAST_COMPRESSION TOAST_PGLZ_COMPRESSION
+#endif
/* pglz compression/decompression routines */
extern struct varlena *pglz_compress_datum(const struct varlena *value);
--
2.39.5
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
@ 2025-12-05 02:23 ` Michael Paquier <[email protected]>
2026-02-27 01:10 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2025-12-05 02:23 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Peter Eisentraut <[email protected]>
On Thu, Dec 04, 2025 at 05:11:15PM -0300, Euler Taveira wrote:
> Here it is v2. It enforces the lz4 dependency. It works the same as other
> dependencies (readline, icu, zlib); error if the dependency could not be found.
> Regarding meson, I'm confused. If any of the referred dependencies (icu,
> readline, zlib) is not found, there is no hard error. Instead, the feature is
> disabled. I searched for a discussion about this decision but couldn't find.
> For this patch, I decided to use the same pattern (no error) but I added a
> warning message (similar to zlib).
Another thing to be careful of is that this would immediately break
the CI task CompilerWarnings for mingw:
[02:03:19.626] checking whether to build with LZ4 support... yes
[02:03:19.626] checking for liblz4... no
[02:03:19.694] configure: error: Package requirements (liblz4) were
not met:
So this had better be adjusted in one go, in the shape of a tweak in
mingw_cross_warning_script with the addition of a --without-lz4, same
way as for ICU.
> Should it report an error (like autoconf) instead of silently disable the
> feature that is a requirement? If yes, then we should add error messages for
> these 3 dependencies.
Hmm. It seems to me that we should just set not_found_dep if lz4
cannot be found, leaving meson_options.txt as it is currently, no?
One could always force meson's hand with -Dlz4=disabled. As a whole,
I'd value more consistency with how icu is handled if we want to force
LZ4 across the board in the backend. The case with zlib is different
in the backend: we only use it on a wanted-basis for compression
specifications in server-side compressed base backups, so it seems to
me that there's a case for being a tad more aggressive with LZ4 as it
relates to TOAST compression, forcing an error rather than ignoring it
silently if it cannot be found.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-12-05 02:23 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
@ 2026-02-27 01:10 ` Michael Paquier <[email protected]>
2026-03-04 04:09 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2026-02-27 01:10 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Euler Taveira <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On Tue, Feb 24, 2026 at 10:52:39PM +0100, Peter Eisentraut wrote:
> Your patch is adding documentation for the configure option --without-lz4,
> but you are leaving the documentation for --with-lz4 in place. You should
> delete the latter.
Ah, right. This is what we have done for ICU in fcb21b3acdcb. While
--with-lz4 is still allowed for backward-compatibility, we show only
--without-icu in the output of configure --help. Reflecting the
documentation to show up the same contents as --help makes sense.
Could you update the patch, Euler? This includes the blip in
meson.build regarding the addition of not_found_dep not really
required.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-12-05 02:23 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-02-27 01:10 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
@ 2026-03-04 04:09 ` Michael Paquier <[email protected]>
2026-03-04 07:15 ` Re: change default default_toast_compression to lz4? Jelte Fennema-Nio <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2026-03-04 04:09 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Euler Taveira <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On Fri, Feb 27, 2026 at 10:10:20AM +0900, Michael Paquier wrote:
> Ah, right. This is what we have done for ICU in fcb21b3acdcb. While
> --with-lz4 is still allowed for backward-compatibility, we show only
> --without-icu in the output of configure --help. Reflecting the
> documentation to show up the same contents as --help makes sense.
It would have been a waste to not see this change happening, so I have
looked a bit more in depth at it. You are right that we should remove
the reference to --with-lz4, but it seems to me that this should apply
to all the references of the switch we have in the docs. The change
in meson.build was not necessary. Another thing not yet included was
the mandatory autoreconf. With all that done, applied.
Now, let's see how much this breaks the buildfarm.. Because things
will break. I'll send a poke to the buildfarm mailing list.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: change default default_toast_compression to lz4?
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-12-05 02:23 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-02-27 01:10 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-03-04 04:09 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
@ 2026-03-04 07:15 ` Jelte Fennema-Nio <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Jelte Fennema-Nio @ 2026-03-04 07:15 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>; Euler Taveira <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On Wed, 4 Mar 2026 at 06:05, Tom Lane <[email protected]> wrote:
>
> Michael Paquier <[email protected]> writes:
> > Now, let's see how much this abreaks the buildfarm.. Because things
> > will break. I'll send a poke to the buildfarm mailing list.
>
> I don't think this idea is really fully baked yet. What happens
> if I build with liblz4, and run for a while with that so that
> there are lz4-compressed TOAST values, and then try to update
> to a build without LZ4?
That doesn't seem like a new problem to me. That's already possible
since PG14. To me it actually seems more likely to happen, because we
default to allowing builds without lz4 (which we stopped doing on
master). The fact that we haven't had (many) people complain indicates
to me that it isn't a big problem in practice.
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2026-03-04 07:15 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 03:11 [PATCH v9 4/8] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]>
2025-11-21 11:11 change default default_toast_compression to lz4? Peter Eisentraut <[email protected]>
2025-11-21 11:33 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-11-21 12:27 ` Re: change default default_toast_compression to lz4? Daniel Gustafsson <[email protected]>
2025-11-22 13:11 ` Re: change default default_toast_compression to lz4? Álvaro Herrera <[email protected]>
2025-11-21 16:10 ` Re: change default default_toast_compression to lz4? Andres Freund <[email protected]>
2025-11-21 16:16 ` Re: change default default_toast_compression to lz4? Tom Lane <[email protected]>
2025-11-26 04:35 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2025-11-26 18:59 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-11-27 09:44 ` Re: change default default_toast_compression to lz4? Aleksander Alekseev <[email protected]>
2025-12-04 20:11 ` Re: change default default_toast_compression to lz4? Euler Taveira <[email protected]>
2025-12-05 02:23 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-02-27 01:10 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-03-04 04:09 ` Re: change default default_toast_compression to lz4? Michael Paquier <[email protected]>
2026-03-04 07:15 ` Re: change default default_toast_compression to lz4? Jelte Fennema-Nio <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox