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

On Wed, Jun 28, 2023 at 09:00:00AM +0300, Alexander Lakhin wrote:
> There is also another scenario where the new behavior could be
> considered as more sensible:
> create table t(a int, b int) partition by range (a);
> create index on t((a / b));
> create table tp1(a int, b int);
> insert into tp1 values (1, 0);
> create index concurrently on tp1((a/b)); --  division by zero occurs, but the index is created (as invalid)
> alter table t attach partition tp1 for values from (1) to (10);
>
> Without the fix you get partition tp1_1 attached and the following partition indexes:
> Partitioned index "public.t_expr_idx"
> btree, for table "public.t"
> Partitions: tp1_expr_idx
>
> Index "public.tp1_expr_idx"
> btree, for table "public.tp1", invalid
>
> But with the patch applied ATTACH PARTITION fails with ERROR: division by zero.

This makes rather sense to me.  You cannot really attach the index
in this case.  And the partition trees are clean.

> Though we still can get a partition index chain with invalid indexes
> as follows:
> create table t(a int, b int) partition by range (a);
> create table tp1(a int, b int) partition by range (a);
> alter table t attach partition tp1 for values from (1) to (100);
> create table tp1_1(a int, b int);
> insert into tp1_1 values (1, 0);
> create index concurrently on tp1_1((a/b)); -- division by zero occurs, but the index is created (as invalid)
> alter table tp1 attach partition tp1_1 for values from (1) to (10);
> create index on t((a / b));

Exotic case, leading to this tree:
=# select indisvalid, relid, parentrelid, isleaf, level
   from pg_partition_tree('t_expr_idx'), pg_index where indexrelid = relid;
 indisvalid |     relid      | parentrelid  | isleaf | level
------------+----------------+--------------+--------+-------
 t          | t_expr_idx     | null         | f      |     0
 f          | tp1_expr_idx   | t_expr_idx   | f      |     1
 f          | tp1_1_expr_idx | tp1_expr_idx | t      |     2
(3 rows)

This looks like a second bug to me in DefineIndex() where we shouldn't
even try to link to the invalid index tp1_1_expr_idx, and we should
try to create a new index instead.  Repeating the last CREATE INDEX
command leads to a failure, which is what I'd expect, because we get
the computation failure.

> It's also interesting that REINDEX for the index tree validates only
> a leaf index:
> reindex index t_expr_idx;
> ERROR:  division by zero

REINDEX on a partitioned index only works on the partitions, it does
nothing for the partitioned indexes.

> So it's not clear (to me, at least), what exactly indisvalid means
> for indexes in a partition tree.

It seems to me that validatePartitionedIndex() is what defines the
use of the flag for partitioned indexes.  indisvalid = false for a
partitioned index means that at least one of its partitions *may* not
have a valid index mapping to it, and that it should be switched to
true once we are sure that all its partitions have a valid index.
indisvalid = true on a partitioned table has much a stronger meaning,
where all the partitions *have* to have valid indexes.  Your first
case would follow that, but not the second.

For now, I have applied a patch down to v11 to fix the first issue for
the partition attaching.  Digging into the second one..
--
Michael