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?
Later we may do constraint checks in ``foreach(l, tab->constraints)``. putting it after table_tuple_insert would be more appropriate, IMHO.
As I understand it, if we get an error from any of the constraints, the ALTER fails anyway so there seems to be no window for mismatch. But it might make sense to move it into the same block where `table_tuple_insert` is anyway.
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.
There are three different branches under it, two of them containing `ATRewriteTable()` calls. So I picked a place before that branching instead of starting in two separate places. It seems simpler but thats my only argument so I am open to other opinions (for both this and where to put the "tuples_written" update).
