diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2023-03-25 15:33:56 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2023-03-25 15:34:03 -0400 |
| commit | 27f5c712b2c57d1c676fbf110705ac881057b57e (patch) | |
| tree | 99552922b9fc6d9bc61172d6c16bdcbff775a747 /src/backend/tcop/utility.c | |
| parent | 81a6d57e33515958203938cb686bf0c255255cff (diff) | |
Fix CREATE INDEX progress reporting for multi-level partitioning.
The "partitions_total" and "partitions_done" fields were updated
as though the current level of partitioning was the only one.
In multi-level cases, not only could partitions_total change
over the course of the command, but partitions_done could go
backwards or exceed the currently-reported partitions_total.
Fix by setting partitions_total to the total number of direct
and indirect children once at command start, and then just
incrementing partitions_done at appropriate points. Invent
a new progress monitoring function "pgstat_progress_incr_param"
to simplify doing the latter. We can avoid adding cost for the
former when doing CREATE INDEX, because ProcessUtility already
enumerates the children and it's pretty easy to pass the count
down to DefineIndex. In principle the same could be done in
ALTER TABLE, but that's structurally difficult; for now, just
eat the cost of an extra find_all_inheritors scan in that case.
Ilya Gladyshev and Justin Pryzby
Discussion: https://postgr.es/m/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel@gmail.com
Diffstat (limited to 'src/backend/tcop/utility.c')
| -rw-r--r-- | src/backend/tcop/utility.c | 11 |
1 files changed, 9 insertions, 2 deletions
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index eada7353639..30b51bf4d30 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1464,6 +1464,7 @@ ProcessUtilitySlow(ParseState *pstate, IndexStmt *stmt = (IndexStmt *) parsetree; Oid relid; LOCKMODE lockmode; + int nparts = -1; bool is_alter_table; if (stmt->concurrent) @@ -1494,7 +1495,9 @@ ProcessUtilitySlow(ParseState *pstate, * * We also take the opportunity to verify that all * partitions are something we can put an index on, to - * avoid building some indexes only to fail later. + * avoid building some indexes only to fail later. While + * at it, also count the partitions, so that DefineIndex + * needn't do a duplicative find_all_inheritors search. */ if (stmt->relation->inh && get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE) @@ -1505,7 +1508,8 @@ ProcessUtilitySlow(ParseState *pstate, inheritors = find_all_inheritors(relid, lockmode, NULL); foreach(lc, inheritors) { - char relkind = get_rel_relkind(lfirst_oid(lc)); + Oid partrelid = lfirst_oid(lc); + char relkind = get_rel_relkind(partrelid); if (relkind != RELKIND_RELATION && relkind != RELKIND_MATVIEW && @@ -1523,6 +1527,8 @@ ProcessUtilitySlow(ParseState *pstate, errdetail("Table \"%s\" contains partitions that are foreign tables.", stmt->relation->relname))); } + /* count direct and indirect children, but not rel */ + nparts = list_length(inheritors) - 1; list_free(inheritors); } @@ -1548,6 +1554,7 @@ ProcessUtilitySlow(ParseState *pstate, InvalidOid, /* no predefined OID */ InvalidOid, /* no parent index */ InvalidOid, /* no parent constraint */ + nparts, /* # of partitions, or -1 */ is_alter_table, true, /* check_rights */ true, /* check_not_in_use */ |