Re: Parent index on partitioned table could still be invalid even when children are not

I've done a bit more snooping around.

"ALTER INDEX .. ATTACH PARTITION ..." calls validatePartitionedIndex
<https://github.com/postgres/postgres/blob/01463e1cccd33fb11b33a4dd6dbebcad3c534102/src/backend/commands/tablecmds.c#L20751>
and
this is what "heals" the parent index when you've attached enough
partitions to it. But this function is not called anywhere in the reindex
code.

I've tried to add it to both REINDEX and REINDEX CONCURRENTLY code path on
the REL_13_15 branch (this is the earliest version that I have around that
has the behaviour that I am trying to alter, and it also has simpler
ReindexIndex code than later versions), and confirmed that even this clumsy
attempt makes REINDEX and REINDEX CONCURRENTLY on the partition of an index
correctly mark the parent index as valid once all children are valid.

I am attaching the patch that I used for this experiment. I am sure that it
is very crude and the proper fix could be done much better by someone more
familiar with the codebase, but I hope that it might still be useful - if
only as a proof-of-concept.

Best regards, Dmytro

On Thu, Jan 23, 2025 at 7:09 PM Dmytro Astapov <dastapov(at)gmail(dot)com> wrote:

> OS: Debian, Rock Linux
> Postgres versions: 13.6, 15.6, 17.0
>
> Setup:
>
> create table tbl(id int, d date, payload text) partition by range(d);
> create table tbl_2023 partition of tbl for values from ('2023-01-01') to
> ('2024-01-01');
> create table tbl_2024 partition of tbl for values from ('2024-01-01') to
> ('2025-01-01');
>
> -- Let's assume that the tbl is very large, as are the partitions,
> -- so we are adding a new index by building it concurrently on partitions
> and then creating
> -- it on the parent table, which should attach partition indexes
> create unique index concurrently on tbl_2023(d,id);
> create unique index concurrently on tbl_2024(d,id);
>
> -- Index on tbl_2024 ended up being invalid for whatever reason - in our
> specific case it was deadlock
> -- I am going to simulate this by marking index as invalid in pg_index
> update pg_index set indisvalid='f' where indexrelid
> ='tbl_2024_d_id_idx'::regclass;
>
> -- We failed to notice that partition index is invalid, and built parent
> index anyway.
> -- It was marked as invalid as well, so now we have two invalid indexes
> create unique index on tbl(d,id);
> select 'before', indexrelid::regclass from pg_index where not indisvalid ;
> /* this outputs:
> ?column? indexrelid
> before tbl_2024_d_id_idx
> before tbl_d_id_idx
> */
>
> -- We can fix the failed index on the partition with a simple reindex
> reindex index concurrently tbl_2024_d_id_idx;
>
> -- ... but this does not fix the index on parent, which is still invalid,
> -- and there does not seem a way to make it valid
> select 'after', indexrelid::regclass from pg_index where not indisvalid ;
> /* this outputs:
> ?column? indexrelid
> after tbl_d_id_idx
> */
>
>
> At this point, there seems to be no way to make the parent index be valid.
> It either can't be reindexed (in v13), or reindex does not mark it as
> valid (v17). `reindex table tbl` does not mark it as valid either.
>
> The only solution I was able to find is to detach all the partitions, drop
> the index (reindex does not work at this stage either), create an index
> again, and reattach all partitions:
>
> alter table tbl detach partition tbl_2023;
> alter table tbl detach partition tbl_2024;
> drop index tbl_d_id_idx;
> create unique index on tbl(d,id);
> alter table tbl attach partition tbl_2023 for values from ('2023-01-01')
> to ('2024-01-01');
> alter table tbl attach partition tbl_2024 for values from ('2024-01-01')
> to ('2025-01-01');
>
> Alternatively, one can also DROP INDEX tbl_d_id_idx, which would drop all
> index partitions, and start from scratch.
>
> Here is db-fiddle for yor convenience if you want to play with it:
> https://www.db-fiddle.com/f/b67c8CfQECbTpk3RXkXnPr/0
>
> Unfortunately, in my practical case the tables involved are very large, so
> detaching/reattaching partitions, or re-doing the index creation would lead
> to couple of days of extra work and runtime.
>
> I think the code involved here is contained in the
> backend/commands/indexcmds.c
> When the parent index is created, we arrive to this line
> <https://github.com/postgres/postgres/blob/01463e1cccd33fb11b33a4dd6dbebcad3c534102/src/backend/commands/indexcmds.c#L1268>
> in DefineIndex , where we would go over all indexes on partitions in search
> for a matching one.
>
> If a match is found, it would be attached to the parent, which happens
> here
> <https://github.com/postgres/postgres/blob/01463e1cccd33fb11b33a4dd6dbebcad3c534102/src/backend/commands/indexcmds.c#L1421-L1441>.
> However, if it is invalid, then invalidate_parent is set to true, and this
> later triggers invalidation of the parent index a dozen of lines later,
> here
> <https://github.com/postgres/postgres/blob/01463e1cccd33fb11b33a4dd6dbebcad3c534102/src/backend/commands/indexcmds.c#L1524-L1544>.
> There does not seem to be a code path (outside of ALTER INDEX ... ATTACH
> PARTITION ...) that could mark the parent index valid again.
>
> This feels like a bug, with the expected behaviour being "once the last
> invalid partition of an index becomes valid (and no partitions are
> missing), the partitioned index should become valid as well". Would you
> agree?
>
> Also, is it naive to think that in this specific case marking the parent
> index as valid via pg_index update might be warranted as a solution?
>
> --
> Best regards, Dmytro
>