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

[jian he]

On Mon, Jun 2, 2025 at 3:35 PM Jiří Kavalík <jkava...@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.