Re: REINDEX not updating partition progress

From: Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: REINDEX not updating partition progress
Date: 2024-07-25 21:21:49
Message-ID: 8f487a28-a559-4fed-bffc-54a92a201fe9@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 25 июля 2024 г., в 09:55, Michael Paquier <michael(at)paquier(dot)xyz>
> написал(а):
>
> On Sun, Jul 21, 2024 at 11:41:43AM +0100, Ilya Gladyshev wrote:
>> Forgot to update partition_relid in reindex_index in the second
>> patch. Fixed in attachment.
>
>        <structfield>relid</structfield> <type>oid</type>
>       </para>
>       <para>
> -       OID of the table on which the index is being created.
> +       OID of the table on which the command was run.
>       </para></entry>
>
> Hmm.  I am not sure if we really need to change the definition of this
> field, because it can have the same meaning when using a REINDEX on a
> partitioned table, pointing to the parent table (the partition) of the
> index currently rebuilt.
>
> Hence, rather than a partition_relid, could a partitioned_relid
> reflect better the situation, set only when issuing a REINDEX on a
> partitioned relation?

I'm not quite happy with the documentation update, but I think the
approach for partitioned tables in this patch makes sense. I checked
what other commands, that deal with partitions, (CREATE INDEX and
ANALYZE) do, and they put a root partitioned table in "relid". ANALYZE
has a separate column for the id of partition named
current_child_table_relid, so I think it makes sense to have REINDEX do
the same.

In addition, the current API for progress tracking doesn't have a way of
updating "relid" without wiping out all other fields (that's what
pgstat_progress_start_command does). This can definitely be changed, but
that's another thing that made me not think in this direction.

> +   if (relkind == RELKIND_PARTITIONED_INDEX)
> +   {
> +       heapId = IndexGetRelation(relid, true);
> +   }
> Shouldn't we report the partitioned index OID rather than its parent
> table when running a REINDEX on a partitioned index?
> —
> Michael

It’s used to update the "relid" field of the progress report. It’s the
one that’s described in docs currently as "OID of the table on which the
index is being created.", so I think it’s correct.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2024-07-25 21:52:41 Re: Protocol question regarding Portal vs Cursor
Previous Message Tom Lane 2024-07-25 21:14:13 Re: Recent 027_streaming_regress.pl hangs