public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v8 2/8] Implement CLUSTER of partitioned table..
6+ messages / 4 participants
[nested] [flat]
* [PATCH v8 2/8] Implement CLUSTER of partitioned table..
@ 2020-06-07 21:58 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Justin Pryzby @ 2020-06-07 21:58 UTC (permalink / raw)
This requires either specification of a partitioned index on which to cluster,
or that an partitioned index was previously set clustered.
---
doc/src/sgml/ref/cluster.sgml | 6 +
src/backend/commands/cluster.c | 174 +++++++++++++++++++-------
src/bin/psql/tab-complete.c | 1 +
src/include/commands/cluster.h | 1 +
src/test/regress/expected/cluster.out | 58 ++++++++-
src/test/regress/sql/cluster.sql | 24 +++-
6 files changed, 209 insertions(+), 55 deletions(-)
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 5dd21a0189..fb5deddb35 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -192,6 +192,12 @@ CLUSTER [VERBOSE]
are periodically reclustered.
</para>
+ <para>
+ Clustering a partitioned table clusters each of its partitions using the
+ index partition of the given partitioned index or (if not specified) the
+ partitioned index marked as clustered.
+ </para>
+
</refsect1>
<refsect1>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 096a06f7b3..9b6673867c 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -32,7 +32,9 @@
#include "catalog/index.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
+#include "catalog/pg_inherits.h"
#include "catalog/toasting.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
@@ -73,6 +75,9 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
static List *get_tables_to_cluster(MemoryContext cluster_context);
+static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
+ Oid indexOid);
+static void cluster_multiple_rels(List *rvs, int options);
/*---------------------------------------------------------------------------
@@ -135,7 +140,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
AccessExclusiveLock,
0,
RangeVarCallbackOwnsTable, NULL);
- rel = table_open(tableOid, NoLock);
+ rel = table_open(tableOid, ShareUpdateExclusiveLock);
/*
* Reject clustering a remote temp table ... their local buffer
@@ -146,14 +151,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot cluster temporary tables of other sessions")));
- /*
- * Reject clustering a partitioned table.
- */
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot cluster a partitioned table")));
-
if (stmt->indexname == NULL)
{
ListCell *index;
@@ -189,10 +186,34 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
}
/* close relation, keep lock till commit */
- table_close(rel, NoLock);
+ table_close(rel, ShareUpdateExclusiveLock);
- /* Do the job. */
- cluster_rel(tableOid, indexOid, ¶ms);
+ if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ {
+ /* Do the job. */
+ cluster_rel(tableOid, indexOid, ¶ms);
+ }
+ else
+ {
+ List *rvs;
+ MemoryContext cluster_context;
+
+ /* Refuse to hold strong locks in a user transaction */
+ PreventInTransactionBlock(isTopLevel, "CLUSTER");
+
+ cluster_context = AllocSetContextCreate(PortalContext,
+ "Cluster",
+ ALLOCSET_DEFAULT_SIZES);
+
+ rvs = get_tables_to_cluster_partitioned(cluster_context, indexOid);
+ cluster_multiple_rels(rvs, params.options);
+
+ /* Start a new transaction for the cleanup work. */
+ StartTransactionCommand();
+
+ /* Clean up working storage */
+ MemoryContextDelete(cluster_context);
+ }
}
else
{
@@ -202,7 +223,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
*/
MemoryContext cluster_context;
List *rvs;
- ListCell *rv;
/*
* We cannot run this form of CLUSTER inside a user transaction block;
@@ -225,28 +245,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
* cluster_context.
*/
rvs = get_tables_to_cluster(cluster_context);
-
- /* Commit to get out of starting transaction */
- PopActiveSnapshot();
- CommitTransactionCommand();
-
- /* Ok, now that we've got them all, cluster them one by one */
- foreach(rv, rvs)
- {
- RelToCluster *rvtc = (RelToCluster *) lfirst(rv);
- ClusterParams cluster_params = params;
-
- /* Start a new transaction for each relation. */
- StartTransactionCommand();
- /* functions in indexes may want a snapshot set */
- PushActiveSnapshot(GetTransactionSnapshot());
- /* Do the job. */
- cluster_params.options |= CLUOPT_RECHECK;
- cluster_rel(rvtc->tableOid, rvtc->indexOid,
- &cluster_params);
- PopActiveSnapshot();
- CommitTransactionCommand();
- }
+ cluster_multiple_rels(rvs, params.options | CLUOPT_RECHECK_ISCLUSTERED);
/* Start a new transaction for the cleanup work. */
StartTransactionCommand();
@@ -352,9 +351,10 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
}
/*
- * Check that the index is still the one with indisclustered set.
+ * Check that the index is still the one with indisclustered set, if needed.
*/
- if (!get_index_isclustered(indexOid))
+ if ((params->options & CLUOPT_RECHECK_ISCLUSTERED) != 0 &&
+ !get_index_isclustered(indexOid))
{
relation_close(OldHeap, AccessExclusiveLock);
pgstat_progress_end_command();
@@ -398,8 +398,13 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
/* Check heap and index are valid to cluster on */
if (OidIsValid(indexOid))
+ {
check_index_is_clusterable(OldHeap, indexOid, recheck, AccessExclusiveLock);
+ /* Mark the index as clustered */
+ mark_index_clustered(OldHeap, indexOid, true);
+ }
+
/*
* Quietly ignore the request if this is a materialized view which has not
* been populated from its query. No harm is done because there is no data
@@ -415,6 +420,14 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
return;
}
+ /* For a partitioned rel, we're done. */
+ if (!RELKIND_HAS_STORAGE(get_rel_relkind(tableOid)))
+ {
+ relation_close(OldHeap, AccessExclusiveLock);
+ pgstat_progress_end_command();
+ return;
+ }
+
/*
* All predicate locks on the tuples or pages are about to be made
* invalid, because we move tuples around. Promote them to relation
@@ -483,6 +496,9 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
* the worst consequence of following broken HOT chains would be that we
* might put recently-dead tuples out-of-order in the new table, and there
* is little harm in that.)
+ *
+ * This also refuses to cluster on an "incomplete" partitioned index
+ * created with "ON ONLY".
*/
if (!OldIndex->rd_index->indisvalid)
ereport(ERROR,
@@ -507,12 +523,6 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
Relation pg_index;
ListCell *index;
- /* Disallow applying to a partitioned table */
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot mark index clustered in partitioned table")));
-
/*
* If the index is already marked clustered, no need to do anything.
*/
@@ -584,10 +594,6 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
TransactionId frozenXid;
MultiXactId cutoffMulti;
- /* Mark the correct index as clustered */
- if (OidIsValid(indexOid))
- mark_index_clustered(OldHeap, indexOid, true);
-
/* Remember info about rel before closing OldHeap */
relpersistence = OldHeap->rd_rel->relpersistence;
is_system_catalog = IsSystemRelation(OldHeap);
@@ -1582,3 +1588,77 @@ get_tables_to_cluster(MemoryContext cluster_context)
return rvs;
}
+
+/*
+ * Return a List of tables and associated index, where each index is a
+ * partition of the given index
+ */
+static List *
+get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
+{
+ List *inhoids;
+ ListCell *lc;
+ List *rvs = NIL;
+ MemoryContext old_context;
+
+ inhoids = find_all_inheritors(indexOid, NoLock, NULL);
+
+ foreach(lc, inhoids)
+ {
+ Oid indexrelid = lfirst_oid(lc);
+ Oid relid = IndexGetRelation(indexrelid, false);
+ RelToCluster *rvtc;
+
+ /*
+ * Partitioned rels are also processed by cluster_rel, to
+ * call check_index_is_clusterable() and mark_index_clustered().
+ */
+
+ /*
+ * We have to build the list in a different memory context so it will
+ * survive the cross-transaction processing
+ */
+ old_context = MemoryContextSwitchTo(cluster_context);
+
+ rvtc = (RelToCluster *) palloc(sizeof(RelToCluster));
+ rvtc->tableOid = relid;
+ rvtc->indexOid = indexrelid;
+ rvs = lappend(rvs, rvtc);
+
+ MemoryContextSwitchTo(old_context);
+ }
+
+ return rvs;
+}
+
+/* Cluster each relation in a separate transaction */
+static void
+cluster_multiple_rels(List *rvs, int options)
+{
+ ListCell *lc;
+
+ /* Commit to get out of starting transaction */
+ PopActiveSnapshot();
+ CommitTransactionCommand();
+
+ /* Ok, now that we've got them all, cluster them one by one */
+ foreach(lc, rvs)
+ {
+ RelToCluster *rvtc = (RelToCluster *) lfirst(lc);
+ ClusterParams cluster_params = { .options = options, };
+
+ /* Start a new transaction for each relation. */
+ StartTransactionCommand();
+
+ /* functions in indexes may want a snapshot set */
+ PushActiveSnapshot(GetTransactionSnapshot());
+
+ /* Do the job. */
+ cluster_params.options |= CLUOPT_RECHECK;
+ cluster_rel(rvtc->tableOid, rvtc->indexOid,
+ &cluster_params);
+
+ PopActiveSnapshot();
+ CommitTransactionCommand();
+ }
+}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 1e1c315bae..ba301a6b08 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -588,6 +588,7 @@ static const SchemaQuery Query_for_list_of_clusterables = {
.catname = "pg_catalog.pg_class c",
.selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
+ CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
CppAsString2(RELKIND_MATVIEW) ")",
.viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
.namespace = "c.relnamespace",
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index a941f2accd..c30ca01726 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -22,6 +22,7 @@
/* flag bits for ClusterParams->flags */
#define CLUOPT_RECHECK 0x01 /* recheck relation state */
#define CLUOPT_VERBOSE 0x02 /* print progress info */
+#define CLUOPT_RECHECK_ISCLUSTERED 0x04 /* recheck relation state for indisclustered */
/* options for CLUSTER */
typedef struct ClusterParams
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index bdae8fe00c..c74cfa88cc 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -439,14 +439,62 @@ select * from clstr_temp;
drop table clstr_temp;
RESET SESSION AUTHORIZATION;
--- Check that partitioned tables cannot be clustered
+-- Check that partitioned tables can be clustered
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
+CREATE TABLE clstrpart1 PARTITION OF clstrpart FOR VALUES FROM (1)TO(10) PARTITION BY RANGE (a);
+CREATE TABLE clstrpart11 PARTITION OF clstrpart1 FOR VALUES FROM (1)TO(10);
+CREATE TABLE clstrpart12 PARTITION OF clstrpart1 FOR VALUES FROM (10)TO(20) PARTITION BY RANGE(a);
+CREATE TABLE clstrpart2 PARTITION OF clstrpart FOR VALUES FROM (20)TO(30);
+CREATE TABLE clstrpart3 PARTITION OF clstrpart DEFAULT PARTITION BY RANGE(a);
+CREATE TABLE clstrpart33 PARTITION OF clstrpart3 DEFAULT;
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+CREATE INDEX clstrpart_only_idx ON ONLY clstrpart (a);
+CLUSTER clstrpart USING clstrpart_only_idx; -- fails
+ERROR: cannot cluster on invalid index "clstrpart_only_idx"
+DROP INDEX clstrpart_only_idx;
CREATE INDEX clstrpart_idx ON clstrpart (a);
-ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+-- Check that clustering sets new relfilenodes:
+CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
CLUSTER clstrpart USING clstrpart_idx;
-ERROR: cannot cluster a partitioned table
-DROP TABLE clstrpart;
+CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
+SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C";
+ relname | level | relkind | ?column?
+-------------+-------+---------+----------
+ clstrpart | 0 | p | t
+ clstrpart1 | 1 | p | t
+ clstrpart11 | 2 | r | f
+ clstrpart12 | 2 | p | t
+ clstrpart2 | 1 | r | f
+ clstrpart3 | 1 | p | t
+ clstrpart33 | 2 | r | f
+(7 rows)
+
+-- Check that clustering sets new indisclustered:
+SELECT relname, relkind, indisclustered FROM pg_partition_tree('clstrpart_idx'::regclass) AS tree JOIN pg_index i ON i.indexrelid=tree.relid JOIN pg_class c ON c.oid=indexrelid ORDER BY relname COLLATE "C";
+ relname | relkind | indisclustered
+-------------------+---------+----------------
+ clstrpart11_a_idx | i | t
+ clstrpart12_a_idx | I | t
+ clstrpart1_a_idx | I | t
+ clstrpart2_a_idx | i | t
+ clstrpart33_a_idx | i | t
+ clstrpart3_a_idx | I | t
+ clstrpart_idx | I | t
+(7 rows)
+
+CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitioned
+CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
+CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
+\d clstrpart
+ Partitioned table "public.clstrpart"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition key: RANGE (a)
+Indexes:
+ "clstrpart_idx" btree (a) CLUSTER
+Number of partitions: 3 (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 188183647c..9bcc77695c 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -196,12 +196,30 @@ drop table clstr_temp;
RESET SESSION AUTHORIZATION;
--- Check that partitioned tables cannot be clustered
+-- Check that partitioned tables can be clustered
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
+CREATE TABLE clstrpart1 PARTITION OF clstrpart FOR VALUES FROM (1)TO(10) PARTITION BY RANGE (a);
+CREATE TABLE clstrpart11 PARTITION OF clstrpart1 FOR VALUES FROM (1)TO(10);
+CREATE TABLE clstrpart12 PARTITION OF clstrpart1 FOR VALUES FROM (10)TO(20) PARTITION BY RANGE(a);
+CREATE TABLE clstrpart2 PARTITION OF clstrpart FOR VALUES FROM (20)TO(30);
+CREATE TABLE clstrpart3 PARTITION OF clstrpart DEFAULT PARTITION BY RANGE(a);
+CREATE TABLE clstrpart33 PARTITION OF clstrpart3 DEFAULT;
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+CREATE INDEX clstrpart_only_idx ON ONLY clstrpart (a);
+CLUSTER clstrpart USING clstrpart_only_idx; -- fails
+DROP INDEX clstrpart_only_idx;
CREATE INDEX clstrpart_idx ON clstrpart (a);
-ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+-- Check that clustering sets new relfilenodes:
+CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
CLUSTER clstrpart USING clstrpart_idx;
-DROP TABLE clstrpart;
+CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
+SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C";
+-- Check that clustering sets new indisclustered:
+SELECT relname, relkind, indisclustered FROM pg_partition_tree('clstrpart_idx'::regclass) AS tree JOIN pg_index i ON i.indexrelid=tree.relid JOIN pg_class c ON c.oid=indexrelid ORDER BY relname COLLATE "C";
+CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitioned
+CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
+CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
+\d clstrpart
-- Test CLUSTER with external tuplesorting
--
2.17.0
--O5XBE6gyVG5Rl6Rj
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v8-0003-f-progress-reporting.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PING] fallocate() causes btrfs to never compress postgresql files
@ 2026-02-06 15:33 Andrew Dunstan <[email protected]>
2026-02-06 18:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Dunstan @ 2026-02-06 15:33 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Thomas Munro <[email protected]>; +Cc: Jakub Wartak <[email protected]>; Bruce Momjian <[email protected]>; Dimitrios Apostolou <[email protected]>; Magnus Hagander <[email protected]>; Tomas Vondra <[email protected]>; [email protected]; Andres Freund <[email protected]>; Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Kyotaro Horiguchi <[email protected]>; David Rowley <[email protected]>
On 2026-02-06 Fr 10:11 AM, Tom Lane wrote:
> Thomas Munro <[email protected]> writes:
>> BTW the rest of the patches will reemerge for master, but for the
>> minimal one back-patched: crake complains about an ABI break due to
>> GUC table changes. Of course adding a GUC to the stable branches is
>> unusual and we discussed the need for it in this case. Is that
>> expected? In what way is it part of the ABI? How would one determine
>> in advance that the ABI checker will complain?
> We have very little experience so far with libabigail, so there's
> not any store of knowledge hereabouts on what it'll complain about.
>
> The message is complaining that sizeof(ConfigureNamesEnum) changed,
> which it did, but I don't see how that value would be visible to
> external modules. So maybe a bug in libabigail?
>
>
I guess because of this in guc_tables.h:
extern PGDLLIMPORT struct config_generic ConfigureNames[];
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PING] fallocate() causes btrfs to never compress postgresql files
2026-02-06 15:33 Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
@ 2026-02-06 18:31 ` Tom Lane <[email protected]>
2026-02-06 20:16 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 20:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Tom Lane @ 2026-02-06 18:31 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jakub Wartak <[email protected]>; Bruce Momjian <[email protected]>; Dimitrios Apostolou <[email protected]>; Magnus Hagander <[email protected]>; Tomas Vondra <[email protected]>; [email protected]; Andres Freund <[email protected]>; Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Kyotaro Horiguchi <[email protected]>; David Rowley <[email protected]>
Andrew Dunstan <[email protected]> writes:
> On 2026-02-06 Fr 10:11 AM, Tom Lane wrote:
>> The message is complaining that sizeof(ConfigureNamesEnum) changed,
>> which it did, but I don't see how that value would be visible to
>> external modules. So maybe a bug in libabigail?
> I guess because of this in guc_tables.h:
> extern PGDLLIMPORT struct config_generic ConfigureNames[];
Evidently, but I don't see how that provides a sizeof() value to
onlookers. Just to be sure, I tried to compile a use of that,
and got
test.c:43:53: error: invalid application of ‘sizeof’ to incomplete type ‘struct config_generic[]’
elog(NOTICE, "sizeof(ConfigureNames) = %zu", sizeof(ConfigureNames));
^
So if this isn't an outright bug, it's at least unhelpful behavior.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PING] fallocate() causes btrfs to never compress postgresql files
2026-02-06 15:33 Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 18:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
@ 2026-02-06 20:16 ` Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Dunstan @ 2026-02-06 20:16 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jakub Wartak <[email protected]>; Bruce Momjian <[email protected]>; Dimitrios Apostolou <[email protected]>; Magnus Hagander <[email protected]>; Tomas Vondra <[email protected]>; [email protected]; Andres Freund <[email protected]>; Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Kyotaro Horiguchi <[email protected]>; David Rowley <[email protected]>
On 2026-02-06 Fr 1:31 PM, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> On 2026-02-06 Fr 10:11 AM, Tom Lane wrote:
>>> The message is complaining that sizeof(ConfigureNamesEnum) changed,
>>> which it did, but I don't see how that value would be visible to
>>> external modules. So maybe a bug in libabigail?
>> I guess because of this in guc_tables.h:
>> extern PGDLLIMPORT struct config_generic ConfigureNames[];
> Evidently, but I don't see how that provides a sizeof() value to
> onlookers. Just to be sure, I tried to compile a use of that,
> and got
>
> test.c:43:53: error: invalid application of ‘sizeof’ to incomplete type ‘struct config_generic[]’
> elog(NOTICE, "sizeof(ConfigureNames) = %zu", sizeof(ConfigureNames));
> ^
>
> So if this isn't an outright bug, it's at least unhelpful behavior.
>
Fair enough. I agree it's not terribly helpful. In any case, maybe we
need to look at my earlier suggestion of adding a suppression file.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PING] fallocate() causes btrfs to never compress postgresql files
2026-02-06 15:33 Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 18:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
@ 2026-02-06 20:31 ` Tom Lane <[email protected]>
2026-02-06 22:07 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Thomas Munro <[email protected]>
1 sibling, 1 reply; 6+ messages in thread
From: Tom Lane @ 2026-02-06 20:31 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jakub Wartak <[email protected]>; Bruce Momjian <[email protected]>; Dimitrios Apostolou <[email protected]>; Magnus Hagander <[email protected]>; Tomas Vondra <[email protected]>; [email protected]; Andres Freund <[email protected]>; Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Kyotaro Horiguchi <[email protected]>; David Rowley <[email protected]>
Andrew Dunstan <[email protected]> writes:
> Fair enough. I agree it's not terribly helpful. In any case, maybe we
> need to look at my earlier suggestion of adding a suppression file.
Perhaps, but since we're getting pretty hard up against the release
freeze deadline, I think the thing to do for today is just to update
.abi-compliance-history. We know that way works.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PING] fallocate() causes btrfs to never compress postgresql files
2026-02-06 15:33 Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 18:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
2026-02-06 20:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
@ 2026-02-06 22:07 ` Thomas Munro <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Munro @ 2026-02-06 22:07 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Jakub Wartak <[email protected]>; Bruce Momjian <[email protected]>; Dimitrios Apostolou <[email protected]>; Magnus Hagander <[email protected]>; Tomas Vondra <[email protected]>; [email protected]; Andres Freund <[email protected]>; Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Kyotaro Horiguchi <[email protected]>; David Rowley <[email protected]>
On Sat, Feb 7, 2026 at 9:31 AM Tom Lane <[email protected]> wrote:
> Andrew Dunstan <[email protected]> writes:
> > Fair enough. I agree it's not terribly helpful. In any case, maybe we
> > need to look at my earlier suggestion of adding a suppression file.
>
> Perhaps, but since we're getting pretty hard up against the release
> freeze deadline, I think the thing to do for today is just to update
> .abi-compliance-history. We know that way works.
So it seems I need to do the attached for 18, and the same again for
17 and 16. Look about right?
I peeked at the libabigail man page, and I don't see any discussion of
incomplete types, but it looks like OPAQUE_TYPE_LABEL might be a way
to tell it about this. If that works, wouldn't it be better than
doing this forever? Where would that be configured?
Attachments:
[text/x-patch] 0001-Tell-ABI-checker-that-33e3de6d-is-OK.patch (1.3K, ../../CA+hUKGJ2yEsjjmq7GZLqT9bfuJjG1EPeqeapmW5RxroVpPThfA@mail.gmail.com/2-0001-Tell-ABI-checker-that-33e3de6d-is-OK.patch)
download | inline diff:
From a209e9c5cfeef91aa597d83854b1d448cf2c7617 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Sat, 7 Feb 2026 10:56:04 +1300
Subject: [PATCH] Tell ABI checker that 33e3de6d is OK.
It's not really an ABI break if you change the layout/size of an object
with incomplete type.
Discussion: https://www.postgresql.org/message-id/1871492.1770409863%40sss.pgh.pa.us
---
.abi-compliance-history | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/.abi-compliance-history b/.abi-compliance-history
index f4e27cfb160..420ad2c3aa1 100644
--- a/.abi-compliance-history
+++ b/.abi-compliance-history
@@ -18,6 +18,16 @@
# Be sure to replace "<ADD JUSTIFICATION HERE>" with details of your change and
# why it is deemed acceptable.
+33e3de6d77e87d6c3c6f8f878dd8de42d37c3b8f
+#
+# Add file_extend_method=posix_fallocate,write_zeros.
+# 2026-02-06 17:38:39 +1300
+#
+# Modifying GUC tables isn't really an ABI break: the relevant object has
+# incomplete type so its layout is inaccessible from C.
+#
+# Discussion: https://www.postgresql.org/message-id/flat/e1f0cd3b-0164-45f5-9705-e922e59df90f%40dunslane.net#8a350b54012c0042f9869d288e978cfe
+
492a69e1407029f8c673484f44aa719a63323d77
#
# Reject ADD CONSTRAINT NOT NULL if name mismatches existing constraint
--
2.52.0
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-02-06 22:07 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-06-07 21:58 [PATCH v8 2/8] Implement CLUSTER of partitioned table.. Justin Pryzby <[email protected]>
2026-02-06 15:33 Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 18:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
2026-02-06 20:16 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Andrew Dunstan <[email protected]>
2026-02-06 20:31 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Tom Lane <[email protected]>
2026-02-06 22:07 ` Re: [PING] fallocate() causes btrfs to never compress postgresql files Thomas Munro <[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