Re: Progress report of CREATE INDEX for nested partitioned tables
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Ilya Gladyshev <ilya.v.gladyshev@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-03-16T00:07:33Z
Lists: pgsql-hackers
Attachments
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...
Commits
-
Fix CREATE INDEX progress reporting for multi-level partitioning.
- 27f5c712b2c5 16.0 landed