Re: Progress report of CREATE INDEX for nested partitioned tables

From: Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Progress report of CREATE INDEX for nested partitioned tables
Date: 2022-12-12 19:39:23
Message-ID: D385C1FC-23C3-464B-B260-EA6398A53BAD@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> Could you check what I've written as a counter-proposal ?

I think that this might be a good solution to start with, it gives us the opportunity to improve the granularity later without any surprising changes for the end user. We could use this patch for previous versions and make more granular output in the latest. What do you think?

> As long as we're changing partitions_done to include nested
> sub-partitions, it seems to me like we should exclude intermediate
> "catalog-only" partitioned indexes, and count only physical leaf
> partitions. Should it alo exclude any children with matching indexes,
> which will also be catalog-only changes? Probably not.
>
> The docs say:
> |When creating an index on a partitioned table, this column is set to the
> |total number of partitions on which the index is to be created. This
> |field is 0 during a REINDEX.

I agree with you on catalog-only partitioned indexes, but I think that in the perfect world we should exclude all the relations where the index isn’t actually created, so that means excluding attached indexes as well. However, IMO doing it this way will require too much of a code rewrite for quite a minor feature (but we could do it, ofc). I actually think that the progress view would be better off without the total number of partitions, but I’m not sure we have this option now. With this in mind, I think your proposal to exclude catalog-only indexes sounds reasonable to me, but I feel like the docs are off in this case, because the attached indexes are not created, but we pretend like they are in this metric, so we should fix one or the other.

>
>> I changed current behaviour to report the total number of partitions in
>> the inheritance tree and fixed recursion in the attached patch. I used
>> a static variable to keep the counter to avoid ABI breakage of
>> DefineIndex, so that we could backpatch this to previous versions.
>
> I wrote a bunch of assertions for this, which seems to have uncovered an
> similar issue with COPY progress reporting, dating to 8a4f618e7. I'm
> not sure the assertions are okay. I imagine they may break other
> extensions, as with file_fdw.
>
> --
> Justin
> <0001-fix-progress-reporting-of-nested-partitioned-indexes.patch>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Oliver Yang 2022-12-12 19:42:55 Re: Why does L&Y Blink Tree need lock coupling?
Previous Message Alvaro Herrera 2022-12-12 19:25:32 Re: PGDOCS - Logical replication GUCs - added some xrefs