Re: Progress report of CREATE INDEX for nested partitioned tables

On Sat, Mar 25, 2023 at 03:43:32PM -0400, Tom Lane wrote: > I pushed 0001 with some cosmetic changes (for instance, trying to > make the style of the doc entries for partitions_total/partitions_done > match the rest of their table). Thanks. > I'm not touching 0002 or 0003, because I think

Re: Progress report of CREATE INDEX for nested partitioned tables

I pushed 0001 with some cosmetic changes (for instance, trying to make the style of the doc entries for partitions_total/partitions_done match the rest of their table). I'm not touching 0002 or 0003, because I think they're fundamentally a bad idea. Progress reporting is inherently inexact,

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > On Sat, Mar 25, 2023 at 11:55:13AM -0400, Tom Lane wrote: >> That's why I wanted list_length() not list_length() - 1. We are >> doing *something* at the top partitioned table, it just doesn't >> involve a table scan, so I don't find this totally unreasonable. >> If you

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sat, Mar 25, 2023 at 11:55:13AM -0400, Tom Lane wrote: > Justin Pryzby writes: > > On Thu, Mar 23, 2023 at 04:35:46PM -0400, Tom Lane wrote: > >> Furthermore, as things stand it's not hard > >> for PARTITIONS_TOTAL to be zero --- there's at least one such case > >> in the regression tests ---

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > On Thu, Mar 23, 2023 at 04:35:46PM -0400, Tom Lane wrote: >> Furthermore, as things stand it's not hard >> for PARTITIONS_TOTAL to be zero --- there's at least one such case >> in the regression tests --- and that seems just weird to me. > I don't know why it'd seem

Re: Progress report of CREATE INDEX for nested partitioned tables

On Thu, Mar 23, 2023 at 04:35:46PM -0400, Tom Lane wrote: > So I'm still pretty desperately unhappy with count_leaf_partitions. > I don't like expending significant cost purely for progress tracking > purposes, I don't like the undocumented assumption that NoLock is > safe, and what's more, if it

Re: Progress report of CREATE INDEX for nested partitioned tables

So I'm still pretty desperately unhappy with count_leaf_partitions. I don't like expending significant cost purely for progress tracking purposes, I don't like the undocumented assumption that NoLock is safe, and what's more, if it is safe then we've already traversed the inheritance tree to lock

Re: Progress report of CREATE INDEX for nested partitioned tables

On Thu, Mar 16, 2023 at 07:04:16PM +0400, Ilya Gladyshev wrote: > > 16 марта 2023 г., в 04:07, Justin Pryzby написал(а): > > > > 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(),

Re: Progress report of CREATE INDEX for nested partitioned tables

> 16 марта 2023 г., в 04:07, Justin Pryzby написал(а): > > 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

Re: Progress report of CREATE INDEX for nested partitioned tables

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

Re: Progress report of CREATE INDEX for nested partitioned tables

> 14 марта 2023 г., в 18:34, Justin Pryzby написал(а): > > On Mon, Mar 13, 2023 at 10:42:59AM -0400, Tom Lane wrote: >> Justin Pryzby 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.

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > 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

Re: Progress report of CREATE INDEX for nested partitioned tables

On Mon, Mar 13, 2023 at 10:42:59AM -0400, Tom Lane wrote: > Justin Pryzby 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 >

Re: Progress report of CREATE INDEX for nested partitioned tables

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

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sun, Mar 12, 2023 at 06:25:13PM -0400, Tom Lane wrote: > Justin Pryzby writes: > > On Sun, Mar 12, 2023 at 04:14:06PM -0400, Tom Lane wrote: > >> Hm. Could we get rid of count_leaf_partitions by doing the work in > >> ProcessUtilitySlow? Or at least passing that OID list forward instead > >>

Re: Progress report of CREATE INDEX for nested partitioned tables

I wrote: > Justin Pryzby writes: >> count_leaf_partitions() is also called for sub-partitions, in the case >> that a matching "partitioned index" already exists, and the progress >> report needs to be incremented by the number of leaves for which indexes >> were ATTACHED. > Can't you increment

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > On Sun, Mar 12, 2023 at 04:14:06PM -0400, Tom Lane wrote: >> Hm. Could we get rid of count_leaf_partitions by doing the work in >> ProcessUtilitySlow? Or at least passing that OID list forward instead >> of recomputing it? > count_leaf_partitions() is called in two

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sun, Mar 12, 2023 at 04:14:06PM -0400, Tom Lane wrote: > Justin Pryzby writes: > > On Fri, Mar 10, 2023 at 03:36:10PM -0500, Tom Lane wrote: > >> I took a look through this. It seems like basically a good solution, > >> but the count_leaf_partitions() function is bothering me, for two > >>

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > On Fri, Mar 10, 2023 at 03:36:10PM -0500, Tom Lane wrote: >> I took a look through this. It seems like basically a good solution, >> but the count_leaf_partitions() function is bothering me, for two >> reasons: > ... find_all_inheritors() will also have been called by >

Re: Progress report of CREATE INDEX for nested partitioned tables

On Fri, Mar 10, 2023 at 03:36:10PM -0500, Tom Lane wrote: > Justin Pryzby writes: > > Update to address a compiler warning in the supplementary patches adding > > assertions. > > I took a look through this. It seems like basically a good solution, > but the count_leaf_partitions() function is

Re: Progress report of CREATE INDEX for nested partitioned tables

Justin Pryzby writes: > Update to address a compiler warning in the supplementary patches adding > assertions. I took a look through this. It seems like basically a good solution, but the count_leaf_partitions() function is bothering me, for two reasons: 1. It seems like a pretty expensive

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, Feb 08, 2023 at 04:40:49PM -0600, Justin Pryzby wrote: > This squishes together 001/2 as the main patch. > I believe it's ready. Update to address a compiler warning in the supplementary patches adding assertions. >From 71427bf7cd9927af04513ba3fe99e481a8ba1f61 Mon Sep 17 00:00:00 2001

Re: Progress report of CREATE INDEX for nested partitioned tables

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 > > wrote: > > > 1 февр. 2023 г., в 20:27, Matthias van de Meent > > > написал(а): > > > > > >> In HEAD we set

Re: Progress report of CREATE INDEX for nested partitioned tables

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 > wrote: > > > > 1 февр. 2023 г., в 20:27, Matthias van de Meent > > написал(а): > > > >> In HEAD we set TOTAL to whatever number partitioned table we're > >> currently

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, 1 Feb 2023 at 18:51, Ilya Gladyshev wrote: > > 1 февр. 2023 г., в 20:27, Matthias van de Meent > написал(а): > >> 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

Re: Progress report of CREATE INDEX for nested partitioned tables

> 1 февр. 2023 г., в 20:27, Matthias van de Meent > написал(а): > > On Wed, 1 Feb 2023 at 16:53, Justin Pryzby > wrote: >> >> On Wed, Feb 01, 2023 at 04:21:35PM +0100, Matthias van de Meent wrote: >>> On Wed, 1 Feb 2023 at 15:21, Ilya Gladyshev >>> wrote:

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, 1 Feb 2023 at 16:53, Justin Pryzby wrote: > > On Wed, Feb 01, 2023 at 04:21:35PM +0100, Matthias van de Meent wrote: > > On Wed, 1 Feb 2023 at 15:21, Ilya Gladyshev > > wrote: > > > > 1 февр. 2023 г., в 16:01, Alvaro Herrera > > > > написал(а): > > > > Hmm, count_leaf_partitions has

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, Feb 01, 2023 at 04:21:35PM +0100, Matthias van de Meent wrote: > On Wed, 1 Feb 2023 at 15:21, Ilya Gladyshev > wrote: > > > 1 февр. 2023 г., в 16:01, Alvaro Herrera > > > написал(а): > > > Hmm, count_leaf_partitions has to scan pg_inherits and do a syscache > > > lookup for every

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, 1 Feb 2023 at 15:21, Ilya Gladyshev wrote: > > > 1 февр. 2023 г., в 16:01, Alvaro Herrera > > написал(а): > > > > Hmm, count_leaf_partitions has to scan pg_inherits and do a syscache > > lookup for every single element therein ... this sounds slow. > > > > In one of the callsites, we

Re: Progress report of CREATE INDEX for nested partitioned tables

> 1 февр. 2023 г., в 16:01, Alvaro Herrera написал(а): > > Hmm, count_leaf_partitions has to scan pg_inherits and do a syscache > lookup for every single element therein ... this sounds slow. > > In one of the callsites, we already have the partition descriptor > available. We could just

Re: Progress report of CREATE INDEX for nested partitioned tables

Hmm, count_leaf_partitions has to scan pg_inherits and do a syscache lookup for every single element therein ... this sounds slow. In one of the callsites, we already have the partition descriptor available. We could just scan partdesc->is_leaf[] and add one for each 'true' value we see there.

Re: Progress report of CREATE INDEX for nested partitioned tables

> 1 февр. 2023 г., в 08:29, Justin Pryzby написал(а): > > On Tue, Jan 31, 2023 at 07:32:20PM +0400, Ilya Gladyshev wrote: >>> 17 янв. 2023 г., в 23:44, Tomas Vondra >>> написал(а): >>> Do we actually need the new parts_done field? I mean, we already do >>> track the value - at

Re: Progress report of CREATE INDEX for nested partitioned tables

On Tue, Jan 31, 2023 at 07:32:20PM +0400, Ilya Gladyshev wrote: > > 17 янв. 2023 г., в 23:44, Tomas Vondra > > написал(а): > > Do we actually need the new parts_done field? I mean, we already do > > track the value - at PROGRESS_CREATEIDX_PARTITIONS_DONE index in the > > st_progress_param array.

Re: Progress report of CREATE INDEX for nested partitioned tables

> 17 янв. 2023 г., в 23:44, Tomas Vondra > написал(а): > Do we actually need the new parts_done field? I mean, we already do > track the value - at PROGRESS_CREATEIDX_PARTITIONS_DONE index in the > st_progress_param array. Can't we simply read it from there? Then we > would not have ABI issues

Re: Progress report of CREATE INDEX for nested partitioned tables

On Wed, 18 Jan 2023 at 15:25, Justin Pryzby wrote: > > TBH, I think the best approach is what I did in: > 0001-report-top-parent-progress-for-CREATE-INDEX.txt > > That's a minimal patch, ideal for backpatching. > > ..which defines/clarifies that the progress reporting is only for > *direct*

Re: Progress report of CREATE INDEX for nested partitioned tables

TBH, I think the best approach is what I did in: 0001-report-top-parent-progress-for-CREATE-INDEX.txt That's a minimal patch, ideal for backpatching. ..which defines/clarifies that the progress reporting is only for *direct* children. That avoids the need to change any data structures, and it's

Re: Progress report of CREATE INDEX for nested partitioned tables

On 1/17/23 21:59, Justin Pryzby wrote: > On Tue, Jan 17, 2023 at 08:44:36PM +0100, Tomas Vondra wrote: >> On 1/9/23 09:44, Ilya Gladyshev wrote: >>> On Sun, 2023-01-08 at 10:48 -0600, Justin Pryzby wrote: On Sat, Dec 17, 2022 at 08:30:02AM -0600, Justin Pryzby wrote: > ... > >

Re: Progress report of CREATE INDEX for nested partitioned tables

On Tue, Jan 17, 2023 at 08:44:36PM +0100, Tomas Vondra wrote: > On 1/9/23 09:44, Ilya Gladyshev wrote: > > On Sun, 2023-01-08 at 10:48 -0600, Justin Pryzby wrote: > >> On Sat, Dec 17, 2022 at 08:30:02AM -0600, Justin Pryzby wrote: > >>> ... > >>> > >>> @committers: Is it okay to add nparts_done to

Re: Progress report of CREATE INDEX for nested partitioned tables

On 1/9/23 09:44, Ilya Gladyshev wrote: > On Sun, 2023-01-08 at 10:48 -0600, Justin Pryzby wrote: >> On Sat, Dec 17, 2022 at 08:30:02AM -0600, Justin Pryzby wrote: >>> ... >>> >>> @committers: Is it okay to add nparts_done to IndexStmt ? >> >> Any hint about this ? >> AFAIK fields added at the end

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sun, 2023-01-08 at 10:48 -0600, Justin Pryzby wrote: > On Sat, Dec 17, 2022 at 08:30:02AM -0600, Justin Pryzby wrote: > > We have the common problem of too many patches. > > > > https://www.postgresql.org/message-id/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel%40gmail.com > > This changes

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sat, Dec 17, 2022 at 08:30:02AM -0600, Justin Pryzby wrote: > We have the common problem of too many patches. > > https://www.postgresql.org/message-id/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel%40gmail.com > This changes the progress reporting to show indirect children as > "total", and

Re: Progress report of CREATE INDEX for nested partitioned tables

On Tue, Dec 13, 2022 at 10:18:58PM +0400, Ilya Gladyshev wrote: > > > I actually think that the progress view would be better off without > > > the total number of partitions, > > > > Just curious - why ? > > We don't really know how many indexes we are going to create, unless we > have some

Re: Progress report of CREATE INDEX for nested partitioned tables

On Mon, 2022-12-12 at 22:43 -0600, Justin Pryzby wrote: > On Mon, Dec 12, 2022 at 11:39:23PM +0400, Ilya Gladyshev wrote: > > > > > Could you check what I've written as a counter-proposal ? > > > > I think that this might be a good solution to start with, it gives > > us the opportunity to

Re: Progress report of CREATE INDEX for nested partitioned tables

On Mon, Dec 12, 2022 at 11:39:23PM +0400, Ilya Gladyshev wrote: > > > Could you check what I've written as a counter-proposal ? > > I think that this might be a good solution to start with, it gives us the > opportunity to improve the granularity later without any surprising changes > for the

Re: Progress report of CREATE INDEX for nested partitioned tables

> Could you check what I've written as a counter-proposal ? I think that this might be a good solution to start with, it gives us the opportunity to improve the granularity later without any surprising changes for the end user. We could use this patch for previous versions and make more

Re: Progress report of CREATE INDEX for nested partitioned tables

On Sat, Dec 10, 2022 at 12:18:32PM +0400, Ilya Gladyshev wrote: > Hi, > > I have noticed that progress reporting for CREATE INDEX of partitioned > tables seems to be working poorly for nested partitioned tables. In > particular, it overwrites total and done partitions count when it > recurses

Progress report of CREATE INDEX for nested partitioned tables

Hi, I have noticed that progress reporting for CREATE INDEX of partitioned tables seems to be working poorly for nested partitioned tables. In particular, it overwrites total and done partitions count when it recurses down to child partitioned tables and it only reports top-level partitions. So