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