Date: Tue, 14 Mar 2023 09:34:16 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Ilya Gladyshev <ilya.v.gladyshev@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Progress report of CREATE INDEX for nested partitioned tables
Message-ID: <ZBCF6CLwLDGNVLm7@telsasoft.com>
References: <20230208224049.GO1653@telsasoft.com>
 <20230217002651.GW1653@telsasoft.com>
 <1000008.1678480570@sss.pgh.pa.us>
 <ZA4xlW9VB+1fYgyo@telsasoft.com>
 <1605789.1678652046@sss.pgh.pa.us>
 <ZA5My7SUI0/VnmCS@telsasoft.com>
 <1621054.1678659913@sss.pgh.pa.us>
 <1634535.1678666528@sss.pgh.pa.us>
 <ZA8qEBqN1Yvd7sCc@telsasoft.com>
 <1760574.1678718579@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1760574.1678718579@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/ZBCF6CLwLDGNVLm7%40telsasoft.com>
Precedence: bulk

On Mon, Mar 13, 2023 at 10:42:59AM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Sun, Mar 12, 2023 at 06:25:13PM -0400, Tom Lane wrote:
> >> I agree that adding such a field to IndexStmt would be a very bad idea.
> >> However, adding another parameter to DefineIndex doesn't seem like a
> >> problem.
> 
> > It's a problem since this is a bug and it's desirable to backpatch a
> > fix, right ?
> 
> I do not think this is important enough to justify a back-patch.

That's fine with me, but it comes as a surprise, and it might invalidate
earlier discussions, which were working under the constraint of
maintaining a compatible ABI.

> > Incrementing by 0 sounds terrible, since someone who has intermediate
> > partitioned tables is likely to always see 0% done.
> 
> How so?  The counter will increase after there's some actual work done,
> ie building an index.  If there's no indexes to build then it hardly
> matters, because the command will complete in very little time.

I misunderstood your idea as suggesting to skip progress reporting for
*every* intermediate partitioned index, and its leaves.  But I guess
what you meant is to skip progress reporting when ATTACHing an
intermediate partitioned index. That seems okay, since 1) intermediate
partitioned tables are probably rare, and 2) ATTACH is fast, so the
effect is indistinguisable from querying the progress report a few
moments later.

The idea would be for:
1) TOTAL to show the number of direct and indirect leaf partitions;
2) update progress while building direct or indirect indexes;
3) ATTACHing intermediate partitioned tables to increment by 0;
4) ATTACHing a direct child should continue to increment by 1,
since that common case already works as expected and shouldn't be
changed.

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().

-- 
Justin