Postgres Pro
Downloads
Home   >   mailing lists

Thread: [PATCH] Support for basic ALTER TABLE progress reporting.

[PATCH] Support for basic ALTER TABLE progress reporting.

From
Jiří Kavalík
Date:
Hi,
while testing int->bigint migrations for our db at work I really missed ALTER TABLE progress reporting and during the waits I checked the code.
It seems to me that ALTERs can be mostly categorized as
1) trivial ones - metadata rewrites, fast adding/removing columns, trying to change column type to one already present etc. not much to report here
2) scanning ones - adding constraints - it imho gives enough info to report blocks total and scanned and tuples scanned
3) rewrites - actually changing data or types - add number of written blocks/tuples
3b) index rewrites - report number of indexes processed

From that it seems to me that the basic info is very similar to already present CLUSTER/VACUUM-FULL reporting so I tried to tap into that and just add a support for a new command.

I identified a handful of places where to add the reporting for ALTERs and it seems to work,

What I changed:
`commands/progress.h`
- new cluster reporting command + new phase for FK checks
 
`commands/tablecmds.c`
- start and end reporting inside `ATRewriteTables()`
- report blocks total, blocks and tuples scanned and possibly tuples written in `ATRewriteTable`
- add at least phase info in `validateForeignKeyConstraint`, possibly more if the check cannot be done by left join

`catalog/system_views.sql`
- output for the new command and phase

`catalog/storage.c`
- number of blocks processed in `RelationCopyStorage()` for the case table is moved between tablespaces by direct copying

+ some basic documentation updates

What I did not have to change - index rebuilds used by CLUSTER reported their progress already, it just was not shown without a valid command configured.

I ran some manual tests locally + ran regression tests and it seems to work fine.

The reporting may be a bit crude and may be missing some phases but it covers the IO-heavy operations with some reasonable numbers. (well, not the FK check by left anti-join, but I don't want to mess with that + maybe number of FKs checked might be shown?)

Thanks
Best regards
jkavalik

Attachment

Re: [PATCH] Support for basic ALTER TABLE progress reporting.

From
jian he
Date:
On Mon, Jun 2, 2025 at 3:35 PM Jiří Kavalík <jkavalik@gmail.com> wrote:
> What I changed:
>
> `commands/tablecmds.c`
> - start and end reporting inside `ATRewriteTables()`
> - report blocks total, blocks and tuples scanned and possibly tuples written in `ATRewriteTable`
> - add at least phase info in `validateForeignKeyConstraint`, possibly more if the check cannot be done by left join
>

hi.
similar to DoCopyTo, processed  as  uint64,
in ATRewriteTable, numTuples should be also uint64 not int?

+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
Later we may do constraint checks in  ``foreach(l, tab->constraints)``.
putting it after table_tuple_insert would be more appropriate, IMHO.

static void
ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
                AlterTableUtilityContext *context)
{
    ListCell   *ltab;
    /* Go through each table that needs to be checked or rewritten */
    foreach(ltab, *wqueue)
    {
        AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
        /* Relations without storage may be ignored here */
        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);
}

Perhaps this is a bit early—we haven't completed the error checks yet.
we have several "ereport(ERROR..." in below.
maybe place it before ATRewriteTable, IMHO.