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 |
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 |