Re: Progress report of CREATE INDEX for nested partitioned tables

> 14 марта 2023 г., в 18:34, Justin Pryzby <pryzby(at)telsasoft(dot)com> написал(а):
>
> On Mon, Mar 13, 2023 at 10:42:59AM -0400, Tom Lane wrote:
>> Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
>>> On Sun, Mar 12, 2023 at 06:25:13PM -0400, Tom Lane wrote:
>>>> 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.
>>
>>> It's a problem since this is a bug and it's desirable to backpatch a
>>> fix, right ?
>>
>> I do not think this is important enough to justify a back-patch.
>
> That's fine with me, but it comes as a surprise, and it might invalidate
> earlier discussions, which were working under the constraint of
> maintaining a compatible ABI.
>
>>> Incrementing by 0 sounds terrible, since someone who has intermediate
>>> partitioned tables is likely to always see 0% done.
>>
>> How so? The counter will increase after there's some actual work done,
>> ie building an index. If there's no indexes to build then it hardly
>> matters, because the command will complete in very little time.
>
> I misunderstood your idea as suggesting to skip progress reporting for
> *every* intermediate partitioned index, and its leaves. But I guess
> what you meant is to skip progress reporting when ATTACHing an
> intermediate partitioned index. That seems okay, since 1) intermediate
> partitioned tables are probably rare, and 2) ATTACH is fast, so the
> effect is indistinguisable from querying the progress report a few
> moments later.

+1 that counting attached partitioned indexes as 0 is fine.

> The idea would be for:
> 1) TOTAL to show the number of direct and indirect leaf partitions;
> 2) update progress while building direct or indirect indexes;
> 3) ATTACHing intermediate partitioned tables to increment by 0;
> 4) ATTACHing a direct child should continue to increment by 1,
> since that common case already works as expected and shouldn't be
> changed.
>
> The only change from the current patch is (3). (1) still calls
> count_leaf_partitions(), but only once. I'd prefer that to rearranging
> the progress reporting to set the TOTAL in ProcessUtilitySlow().
>
> --
> Justin

As for reusing TOTAL calculated outside of DefineIndex, as I can see, ProcessUtilitySlow is not the only call site for DefineIndex (although, I don’t know whether all of them need progress tracking), for instance, there is ALTER TABLE that calls DefineIndex to create index for constraints. So I feel like rearranging progress reporting will result in unnecessary code duplication in those call sites, so passing in an optional parameter seems to be easier here, if we are going to optimize it, after all. Especially if back-patching is a non-issue.