Re: Progress report of CREATE INDEX for nested partitioned tables

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Progress report of CREATE INDEX for nested partitioned tables
Date: 2023-02-02 15:18:07
Message-ID: 20230202151807.GY22427@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 01, 2023 at 07:24:48PM +0100, Matthias van de Meent wrote:
> On Wed, 1 Feb 2023 at 18:51, Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com> wrote:
> >
> > 1 февр. 2023 г., в 20:27, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> написал(а):
> >
> >> In HEAD we set TOTAL to whatever number partitioned table we're
> >> currently processing has - regardless of whether we're the top level
> >> statement.
> >> With the patch we instead add the number of child relations to that
> >> count, for which REL_HAS_STORAGE(child) -- or at least, in the v3
> >> posted by Ilya. Approximately immediately after updating that count we
> >> recurse to the child relations, and that only returns once it is done
> >> creating the indexes, so both TOTAL and DONE go up as we process more
> >> partitions in the hierarchy.
> >
> >
> > The TOTAL in the patch is set only when processing the top-level parent and it is not updated when we recurse, so yes, it is constant. From v3:
>
> Ugh, I misread the patch, more specifically count_leaf_partitions and
> the !OidIsValid(parentIndexId) condition changes.
>
> You are correct, sorry for the noise.

That suggests that the comments could've been more clear. I added a
comment suggested by Tomas and adjusted some others and wrote a commit
message. I even ran pgindent for about the 3rd time ever.

002 are my changes as a separate patch, which you could apply to your
local branch.

And 003/4 are assertions that I wrote to demonstrate the problem and the
verify the fixes, but not being proposed for commit.

--
Justin

Attachment Content-Type Size
0001-create-index-progress-increment.patch text/x-diff 6.6 KB
0002-s-fix-CREATE-INDEX-progress-report-with-nested-parti.patch text/x-diff 6.3 KB
0003-assertions-for-progress-reporting.patch text/x-diff 4.0 KB
0004-f-also-assert-that-progress-values-don-t-go-backward.patch text/x-diff 8.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-02-02 15:26:06 Re: Error "initial slot snapshot too large" in create replication slot
Previous Message Dmitry Dolgov 2023-02-02 15:05:54 Re: pg_stat_statements and "IN" conditions