I did not add CC to the list to my reply so forwarding..
---------- Forwarded message ---------
From: Jiří Kavalík <jkava...@gmail.com>
Date: Sun, Jul 20, 2025 at 8:22 PM
Subject: Re: [PATCH] Support for basic ALTER TABLE progress reporting.
To: jian he <jian.universal...@gmail.com>
Hello,
On Tue, Jul 8, 2025 at 3:42 PM jian he <jian.universal...@gmail.com> wrote:
> hi.
> within ATRewriteTable we have:
> pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
> RelationGetNumberOfBlocks(oldrel));
> pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
> heapScan->rs_nblocks);
>
> PROGRESS_CLUSTER_TOTAL_HEAP_BLKS
> value is fixed, we only need to call pgstat_progress_update_param once
> here?
>
Yes, that was redundant, removed.
>
> another patch [1] is expected to refactor pg_stat_progress_cluster a lot,
> so I'm
> unsure whether it's a good idea to put CLUSTER, VACUUM FULL, or ALTER
> TABLE into
> pg_stat_progress_cluster.
> alternatively, we could introduce a separate progress report specifically
> for
> ALTER TABLE, allowing us to distinguish between table rewrite and table
> scan.
[1] https://commitfest.postgresql.org/patch/5117
I noticed that but not sure if it is targeting v19? I hoped to make the
change as small as possible, but if it would collide with the refactoring
then it makes sense to separate the functionality.
I am attaching the updated patch for the current "minimal" version for now.
But I will look into making it a standalone feature.
Thank you for your insights.
Best regards
jkavalik
From 8d2ca7a6510dc646045cc3be552a61941ff28c6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Kaval=C3=ADk?= <jkavalik@gmail.com>
Date: Sun, 25 May 2025 23:23:56 +0200
Subject: [PATCH] ALTER TABLE progress support
---
doc/src/sgml/monitoring.sgml | 13 +++----
doc/src/sgml/ref/alter_table.sgml | 10 ++++++
src/backend/catalog/storage.c | 7 ++++
src/backend/catalog/system_views.sql | 2 ++
src/backend/commands/tablecmds.c | 53 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 2 ++
src/test/regress/expected/rules.out | 2 ++
7 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 823afe1b30b..02d70a86731 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -400,7 +400,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<row>
<entry><structname>pg_stat_progress_cluster</structname><indexterm><primary>pg_stat_progress_cluster</primary></indexterm></entry>
<entry>One row for each backend running
- <command>CLUSTER</command> or <command>VACUUM FULL</command>, showing current progress.
+ <command>CLUSTER</command>, <command>VACUUM FULL</command> or <command>ALTER TABLE</command>, showing current progress.
See <xref linkend="cluster-progress-reporting"/>.
</entry>
</row>
@@ -5494,7 +5494,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<productname>PostgreSQL</productname> has the ability to report the progress of
certain commands during command execution. Currently, the only commands
which support progress reporting are <command>ANALYZE</command>,
- <command>CLUSTER</command>,
+ <command>CLUSTER</command>, <command>ALTER TABLE</command>,
<command>CREATE INDEX</command>, <command>VACUUM</command>,
<command>COPY</command>,
and <xref linkend="protocol-replication-base-backup"/> (i.e., replication
@@ -5740,8 +5740,9 @@ FROM pg_stat_get_backend_idset() AS backendid;
</indexterm>
<para>
- Whenever <command>CLUSTER</command> or <command>VACUUM FULL</command> is
- running, the <structname>pg_stat_progress_cluster</structname> view will
+ Whenever <command>CLUSTER</command>, <command>VACUUM FULL</command>
+ or <command>ALTER TABLE</command> is running,
+ the <structname>pg_stat_progress_cluster</structname> view will
contain a row for each backend that is currently running either command.
The tables below describe the information that will be reported and
provide information about how to interpret it.
@@ -5803,7 +5804,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>command</structfield> <type>text</type>
</para>
<para>
- The command that is running. Either <literal>CLUSTER</literal> or <literal>VACUUM FULL</literal>.
+ The command that is running. Either <literal>CLUSTER</literal>, <literal>VACUUM FULL</literal> or <literal>ALTER TABLE</literal>.
</para></entry>
</row>
@@ -5886,7 +5887,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</table>
<table id="cluster-phases">
- <title>CLUSTER and VACUUM FULL Phases</title>
+ <title>CLUSTER, VACUUM FULL and ALTER TABLE Phases</title>
<tgroup cols="2">
<colspec colname="col1" colwidth="1*"/>
<colspec colname="col2" colwidth="2*"/>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1e4f26c13f6..865b14ba7d9 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1859,6 +1859,16 @@ ALTER TABLE measurement
</para>
</refsect1>
+ <refsect1>
+ <title>Progress Reporting</title>
+ <para>
+ When an <command>ALTER TABLE</command> operation rewrites the table, progress
+ can be monitored via the <literal>pg_stat_progress_cluster</literal> system view,
+ similar to <command>CLUSTER</command> and <command>VACUUM FULL</command> commands.
+ The command type will be reported as <literal>'ALTER TABLE'</literal>.
+ </para>
+ </refsect1>
+
<refsect1>
<title>See Also</title>
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 227df90f89c..79d14d86bc9 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -26,6 +26,7 @@
#include "access/xlogutils.h"
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/bulk_write.h"
@@ -505,6 +506,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
nblocks = smgrnblocks(src, forkNum);
+ /* Report expected number of block to copy */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, nblocks);
+
for (blkno = 0; blkno < nblocks; blkno++)
{
BulkWriteBuffer buf;
@@ -556,6 +560,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
* page including any unused space.
*/
smgr_bulk_write(bulkstate, blkno, buf, false);
+
+ /* Update progress report */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, blkno + 1);
}
smgr_bulk_finish(bulkstate);
}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index b2d5332effc..9d68d02ec26 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1260,6 +1260,7 @@ CREATE VIEW pg_stat_progress_cluster AS
S.relid AS relid,
CASE S.param1 WHEN 1 THEN 'CLUSTER'
WHEN 2 THEN 'VACUUM FULL'
+ WHEN 3 THEN 'ALTER TABLE'
END AS command,
CASE S.param2 WHEN 0 THEN 'initializing'
WHEN 1 THEN 'seq scanning heap'
@@ -1269,6 +1270,7 @@ CREATE VIEW pg_stat_progress_cluster AS
WHEN 5 THEN 'swapping relation files'
WHEN 6 THEN 'rebuilding index'
WHEN 7 THEN 'performing final cleanup'
+ WHEN 8 THEN 'checking foreign key constraints'
END AS phase,
CAST(S.param3 AS oid) AS cluster_index_relid,
S.param4 AS heap_tuples_scanned,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb811520c29..43376539209 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -59,6 +59,7 @@
#include "commands/comment.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/sequence.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
@@ -5838,6 +5839,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
+ /* Start progress reporting */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tab->relid);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
+
/*
* If we change column data types, the operation has to be propagated
* to tables that use this table's rowtype as a column type.
@@ -5978,6 +5983,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
*/
ATRewriteTable(tab, OIDNewHeap);
+ /* Report that we are now swapping relation files */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES);
/*
* Swap the physical files of the old and new heaps, then rebuild
* indexes and discard the old heap. We can use RecentXmin for
@@ -6089,6 +6097,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
table_close(rel, NoLock);
}
+ /* Report that we are now doing clean up */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP);
+
/* Finally, run any afterStmts that were queued up */
foreach(ltab, *wqueue)
{
@@ -6103,6 +6115,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
CommandCounterIncrement();
}
}
+
+ pgstat_progress_end_command();
}
/*
@@ -6128,6 +6142,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
BulkInsertState bistate;
int ti_options;
ExprState *partqualstate = NULL;
+ int64 numTuples = 0;
/*
* Open the relation(s). We have surely already locked the existing
@@ -6137,6 +6152,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
oldTupDesc = tab->oldDesc;
newTupDesc = RelationGetDescr(oldrel); /* includes all mods */
+ /* Update progress reporting - we are actually scanning and possibly rewriting the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP);
+
if (OidIsValid(OIDNewHeap))
{
Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
@@ -6244,6 +6262,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleTableSlot *oldslot;
TupleTableSlot *newslot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
MemoryContext oldCxt;
List *dropped_attrs = NIL;
ListCell *lc;
@@ -6343,6 +6362,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
*/
snapshot = RegisterSnapshot(GetLatestSnapshot());
scan = table_beginscan(oldrel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
+ heapScan->rs_nblocks);
/*
* Switch to per-tuple memory context and reset it for each tuple
@@ -6353,6 +6375,13 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
if (tab->rewrite > 0)
{
@@ -6401,6 +6430,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ExecStoreVirtualTuple(newslot);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
/*
* Now, evaluate any expressions whose inputs come from the
* new tuple. We assume these columns won't reference each
@@ -6521,6 +6553,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
CHECK_FOR_INTERRUPTS();
}
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ heapScan->rs_nblocks);
MemoryContextSwitchTo(oldCxt);
table_endscan(scan);
@@ -13664,10 +13698,12 @@ validateForeignKeyConstraint(char *conname,
{
TupleTableSlot *slot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
Trigger trig = {0};
Snapshot snapshot;
MemoryContext oldcxt;
MemoryContext perTupCxt;
+ int64 numTuples = 0;
ereport(DEBUG1,
(errmsg_internal("validating foreign key constraint \"%s\"", conname)));
@@ -13686,6 +13722,10 @@ validateForeignKeyConstraint(char *conname,
trig.tginitdeferred = false;
/* we needn't fill in remaining fields */
+ /* Report that we are now checking foreign keys */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_CHECK_FKEYS);
+
/*
* See if we can do it with a single LEFT JOIN query. A false result
* indicates we must proceed with the fire-the-trigger method. We can't do
@@ -13703,6 +13743,7 @@ validateForeignKeyConstraint(char *conname,
snapshot = RegisterSnapshot(GetLatestSnapshot());
slot = table_slot_create(rel, NULL);
scan = table_beginscan(rel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
perTupCxt = AllocSetContextCreate(CurrentMemoryContext,
"validateForeignKeyConstraint",
@@ -13738,6 +13779,14 @@ validateForeignKeyConstraint(char *conname,
RI_FKey_check_ins(fcinfo);
MemoryContextReset(perTupCxt);
+
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
}
MemoryContextSwitchTo(oldcxt);
@@ -16828,6 +16877,10 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
*/
rel = relation_open(tableOid, lockmode);
+ /* Update progress reporting - we are copying the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(rel));
+
/* Check first if relation can be moved to new tablespace */
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
{
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 7c736e7b03b..56585742838 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -74,10 +74,12 @@
#define PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES 5
#define PROGRESS_CLUSTER_PHASE_REBUILD_INDEX 6
#define PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP 7
+#define PROGRESS_CLUSTER_PHASE_CHECK_FKEYS 8
/* Commands of PROGRESS_CLUSTER */
#define PROGRESS_CLUSTER_COMMAND_CLUSTER 1
#define PROGRESS_CLUSTER_COMMAND_VACUUM_FULL 2
+#define PROGRESS_CLUSTER_COMMAND_ALTER_TABLE 3 /* New command type */
/* Progress parameters for CREATE INDEX */
/* 3, 4 and 5 reserved for "waitfor" metrics */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index dce8c672b40..b0d498d0339 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1986,6 +1986,7 @@ pg_stat_progress_cluster| SELECT s.pid,
CASE s.param1
WHEN 1 THEN 'CLUSTER'::text
WHEN 2 THEN 'VACUUM FULL'::text
+ WHEN 3 THEN 'ALTER TABLE'::text
ELSE NULL::text
END AS command,
CASE s.param2
@@ -1997,6 +1998,7 @@ pg_stat_progress_cluster| SELECT s.pid,
WHEN 5 THEN 'swapping relation files'::text
WHEN 6 THEN 'rebuilding index'::text
WHEN 7 THEN 'performing final cleanup'::text
+ WHEN 8 THEN 'checking foreign key constraints'::text
ELSE NULL::text
END AS phase,
(s.param3)::oid AS cluster_index_relid,
--
2.34.1
