Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index

On Thu, Jun 29, 2023 at 12:00:00PM +0300, Alexander Lakhin wrote:
> That is, the partition tree is containing no invalid indexes now, but the
> upper-level indexes in the tree are still invalid.
> Moreover, I don't know how to make them valid:
> reindex index parted_isvalid_idx; / reindex index parted_isvalid_tab_1_expr_idx;
> doesn't affect their flags indisvalid.
> (Though REINDEX for a top-level index can make leaf indexes valid.)
> Reattaching parted_isvalid_tab_11 after "update parted_isvalid_tab_11 set b=1"
> doesn't help either.

DETACH PARTITION is not possible for indexes, but you should be able
to get back to a cleaner tree by detaching a partition, dropping an
index, and re-attaching back the partitions, so as
validatePartitionedIndex() is able to trigger a refresh of
indisvalid depending on the state of the tree. Yeah, that's annoying,
and things are what they are for a few years now. It does not seem
like we've looked at how all that should behave in combination with
concurrent index builds, for example, that can lead to a transient
state of a tree.

> So the above test case makes me wonder, is it realistic in principle to
> maintain this definition of indisvalid up-to-date for a partition
> tree?
>
> So I can't resist asking a question: what would break if indisvalid flags
> were always true (or always false) for the upper-level partition indexes?
> I've made validatePartitionedIndex() a no-op just to check that and the
> only differences I observe doing `make check` are indisvalid flags printed;
> there are no behavioral changes.
> Though `make check-world` fails on src/bin/pg_upgrade (only); I need more
> time to investigate this — maybe more sophisticated query to select
> appropriate indexes for processing could help there.

It offers a hint to the planner about which indexes should be used,
but I would spend a bit more time on plancat.c to see the implications
of that for partitioned indexes. validatePartitionedIndex() has a
clean implied definition, at least.

Anyway, DefineIndex() and its handling of indisvalid is clearly wrong
when it comes to multiple partition layers as it forgets that we may
have to update the parents recursively or we could once again trigger
the assertion from validatePartitionedIndex(), which would not be a
good idea.. So my previous patch fixes that, at least.
--
Michael