Unexpected reindex when altering column types for partitioned tables

Hi,

On 2026-Jun-08, Álvaro Rodríguez wrote:

When running the ALTER TABLE ALTER COLUMN TYPE operation, certain type
changes are supposed to not require a reindex. For example, text to
varchar or varchar(n) to varchar(m) when m > n. However, this turns
out to not be the case for indexes (including pk / unique constraints)
on partitioned tables. Patch 0001 (attached) demonstrates this
behavior with a new regression test. In this case, the table rewrite
is avoided but the indexes are rebuilt.

I agree with the problem statement, and I think the proposed solution
has the right shape. This obviously has to be kept for pg20, so let's
discuss further during the next commitfest; please create a CF entry for
it.

Thanks,

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

Added to CF, thanks Álvaro!

I am also attaching a new version of the patches where we scope the
new constraint recreating behavior to UNIQUE and PK constraints
(instead of everything that is index-backed). This avoids unwanted
impact on how FKs that reference partitioned tables are handled and
should as a bonus fix the CI.

Thanks,
Álvaro

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]/messages/by-id/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com, 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]: /messages/by-id/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Mon Sep 14, 2026 at 8:36 PM CEST, Alberto Piai wrote:

Hi Álvaro,

I started reviewing/testing v2-0002

I forgot to mention: the patch doesn't apply using git am and needs a
rebase (I applied it using patch -p1 after editing the diff). If you're
still interested in working on this, please provide a rebased patch, as
it makes the process smoother :)

Thanks,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

Hey Alberto,

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.

Thanks for the review, and for the suggestion, I'll definitely take a
look at that discussion. At first glance it looks very relevant!

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?

This was my original idea. I kind of discarded it originally because
the two pass approach seemed easier to implement with the current
logic, and I didn't want to mess up too much with the code to minimize
potential side effects. But I forgot that multi-level partitions were
a thing, so we might need to go back and revisit that. It definitely
looked like a valid approach too. I'll see if I can come up with
something.

I forgot to mention: the patch doesn't apply using git am and needs a
rebase (I applied it using patch -p1 after editing the diff). If you're
still interested in working on this, please provide a rebased patch, as
it makes the process smoother :)

Yup! I was aware of that but forgot to update the thread, please find
the rebased versions attached.

Thanks!
Álvaro

Yup! I was aware of that but forgot to update the thread, please find

the rebased versions attached.

Whoops, that was only a partial rebase. v4 attached!
Álvaro

Hi all,

I have a new version of the patch with the following changes:
- The child table recursion now adds children to the queue in
reverse-BFS order (so lowest level first). However, the root table is
added to the work queue elsewhere, which means it is still first.
- We keep the two passes: one of them will do all child indexes (which
will be processed in the same reverse-BFS order), and the next one
will do the root indexes only. This means that everything is processed
in the right order.
- Tests are added to validate that this works on 2-level partitioned tables.

The solution works, but I'm not sure it's final. Ideally we would be
able to move the root table in the work queue to appear after all the
child tables. In that way, we would process everything in the right
order and we wouldn't need the two separate passes. But I'm trying to
figure out where to do so and whether it's safe, since we can't really
delay adding the table to the work queue, but it also seems like a bad
idea to play with the order after creation. The solution above might
be the right balance here.

Additionally, the patch relies on find_all_inheritors() from
pg_inherits.c to return everything in BFS order. This seems to be the
case but isn't documented. The discussion in thread [1]/messages/by-id/57100518-3FF1-48AD-B550-47B9A3E88688@gmail.com may be
relevant for this, and indeed if that ends up merged, we might want to
use their find_all_inheritors_ordered() function to avoid any trouble
here.

Re thread [0]/messages/by-id/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com, I have been doing some testing, I will post something
there to figure out if we can combine the two patches!

Best,
Álvaro

[0]: /messages/by-id/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com
[1]: /messages/by-id/57100518-3FF1-48AD-B550-47B9A3E88688@gmail.com