Re: Progress report of CREATE INDEX for nested partitioned tables

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Matthias van de Meent <boekewurm+postgres(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-03-16 00:07:33
Message-ID: ZBJdxTagTuOZkgov@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 14, 2023 at 06:58:14PM +0400, Ilya Gladyshev wrote:
> > 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().
>
> 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.

Yeah. See attached. I don't like duplicating the loop. Is this really
the right direction to go ?

I haven't verified if the child tables are locked in all the paths which
would call count_leaf_partitions(). But why is it important to lock
them for this? If they weren't locked before, that'd be a pre-existing
problem...

Attachment Content-Type Size
0001-fix-CREATE-INDEX-progress-report-with-nested-partiti.patch text/x-diff 12.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2023-03-16 00:14:24 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Previous Message Peter Smith 2023-03-16 00:03:14 Re: Allow logical replication to copy tables in binary format