Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sun, Mar 12, 2023 at 04:14:06PM -0400, Tom Lane wrote:
>> Hm. Could we get rid of count_leaf_partitions by doing the work in
>> ProcessUtilitySlow? Or at least passing that OID list forward instead
>> of recomputing it?
> count_leaf_partitions() is called in two places:
> Once to get PROGRESS_CREATEIDX_PARTITIONS_TOTAL. It'd be easy enough to
> pass an integer total via IndexStmt (but I think we wanted to avoid
> adding anything there, since it's not a part of the statement).
I agree that adding such a field to IndexStmt would be a very bad idea.
However, adding another parameter to DefineIndex doesn't seem like a
problem. Or could we just call pgstat_progress_update_param directly from
ProcessUtilitySlow, after counting the partitions in the existing loop?
> count_leaf_partitions() is also called for sub-partitions, in the case
> that a matching "partitioned index" already exists, and the progress
> report needs to be incremented by the number of leaves for which indexes
> were ATTACHED.
Can't you increment progress by one at the point where the actual attach
happens?
I also wonder whether leaving non-leaf partitions out of the total
is making things more complicated rather than simpler ...
> We'd need a mapping from OID => npartitions (or to
> compile some data structure of all the partitioned partitions). I guess
> CreateIndex() could call CreatePartitionDirectory(). But it looks like
> that would be *more* expensive.
The reason I find this annoying is that the non-optional nature of the
progress reporting mechanism was sold on the basis that it would add
only negligible overhead. Adding extra pass(es) over pg_inherits
breaks that promise. Maybe it's cheap enough to not matter in the
big scheme of things, but we should not be having to make arguments
like that one.
regards, tom lane