Re: Unexpected reindex when altering column types for partitioned tables

Hi Álvaro,

I started reviewing/testing v2-0002. I think the approach is promising
(more on that later), but I found a problem when testing with multiple
levels of partitions:

create table p (a int, b int generated always as (1) stored)
partition by range (a);
create table m partition of p for values from (1) to (10)
partition by range (a);
create table l partition of m for values from (1) to (5);
create index on p(b);
alter table p alter column b set expression as (2);
ERROR: relation "m_b_idx" already exists

AFICS the problem is that recreating partition indexes in a separate
(later) pass is not enough: in this case both the index on p and the
index on m are partitioned indexes. They will both be enqueued at
AT_PASS_OLD_PARTITIONED_INDEX (each on the AlteredTableInfo
corresponding to its table), but then the tables themselves will be
processed in the wrong order within the AT_PASS_OLD_PARTITIONED_INDEX
pass. The command queue for p will be processed first, which will cause
the index on p(b) to be recreated, which will cause its descendant index
on m(b) to be recreated too. Then m will be processed, and will try to
recreate the same index again.

Maybe a good solution here would be to try and process the tables in the
opposite order when recreating object than the order in which we deleted
them? In which case we (possibly) wouldn't even need a new separate
pass. But I didn't look further into this. What do you think?

Besides this specific problem: there are currently multiple active
threads related to bugs with recursive alter table. I would suggest
specifically to read through [0], all of it because v19 is very
different than v1. The patch coming out of that thread partially
overlaps with this one. It tries to fix properties being lost in
recursive alter table operations that cause index rebuilds, both
properties which could be recreated with alter table commands and
properties which couldn't.

Your patch has potential to fix all properties being lost, which could
be recreated by alter table commands. I have verified for example that
the following is currently broken on master (reported in the other
thread), and your patch (v2) fixes it:

create table p (
a int not null,
b int not null generated always as (1) stored
) partition by range (a);
create table l partition of p for values from (1) to (5);
create unique index on p(a, b);
alter table p replica identity using index p_a_b_idx;
alter table l replica identity using index l_a_b_idx;
comment on index p_a_b_idx is 'comment on p_a_b_idx';
comment on index l_a_b_idx is 'comment on l_a_b_idx';
alter table p alter column b set expression as (2);
-- on master, l_a_b_idx has lost both the comment and the replica
-- identity marker, but not with patch v2 in this thread

I think it's worth exploring your approach too, and possibly joining
efforts with the other thread. The patch in the other thread has very
thorough tests. If your approach worked it could be simplified to do
less work saving all these properties on IndexStmt and restoring them
afterwards, because they would be recreated by enqueuing subcommands.

Kind regards,

Alberto Piai

[0] https://www.postgresql.org/message-id/flat/DB533C25-C6BA-4C0F-8046-96168E9CDD72%40gmail.com

--
Alberto Piai
Sensational AG
Zürich, Switzerland