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-08 22:40:49
Message-ID: 20230208224049.GO1653@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 02, 2023 at 09:18:07AM -0600, Justin Pryzby wrote:
> 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.

That was probably a confusing way to present it - I should've sent the
relative diff as a .txt rather than as patch 002.

This squishes together 001/2 as the main patch.
I believe it's ready.

--
Justin

Attachment Content-Type Size
0001-fix-CREATE-INDEX-progress-report-with-nested-partiti.patch text/x-diff 8.1 KB
0002-assertions-for-progress-reporting.patch text/x-diff 4.0 KB
0003-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 David Rowley 2023-02-08 22:50:37 Re: Can we do something to help stop users mistakenly using force_parallel_mode?
Previous Message Tom Lane 2023-02-08 22:26:26 Re: Can we do something to help stop users mistakenly using force_parallel_mode?