Message-ID: <9390b442-5c09-897f-a58c-b73bef7dd88c@gmail.com>
Date: Fri, 30 Jun 2023 07:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when
 attaching partition index to child of valid index
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
References: <17997-a044c27aef95daf8@postgresql.org>
 <509915.1687704058@sss.pgh.pa.us> <ZJk04dp0mz7s58Lx@paquier.xyz>
 <ZJlG0a/F03iPI/wj@paquier.xyz>
 <14987634-43c0-0cb3-e075-94d423607e08@gmail.com>
 <3eaa2c33-2ed1-2996-c6b6-0e7552a64cae@gmail.com>
 <ZJva9997u5H0RQe1@paquier.xyz>
 <611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com>
 <ZJ1fa/kwMsAvqZit@paquier.xyz>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <ZJ1fa/kwMsAvqZit@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/9390b442-5c09-897f-a58c-b73bef7dd88c%40gmail.com>
Precedence: bulk

29.06.2023 13:39, Michael Paquier wrote:
> 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, I meant DETACH PARTITION for a table. And when I tried the sequence
you have described, I couldn't get a completely valid tree:
alter table parted_isvalid_tab_1 detach partition parted_isvalid_tab_11;
drop index parted_isvalid_idx_11;
update parted_isvalid_tab_11 set b = 1;
alter table parted_isvalid_tab_1 attach partition parted_isvalid_tab_11 for values from (1) to (5);

            indexrelid           | indisvalid | indrelid        |           inhparent
--------------------------------+------------+-----------------------+-------------------------------
  parted_isvalid_idx             | f          | parted_isvalid_tab    |
  parted_isvalid_tab_11_expr_idx | t          | parted_isvalid_tab_11 | parted_isvalid_tab_1_expr_idx
  parted_isvalid_tab_12_expr_idx | t          | parted_isvalid_tab_12 | parted_isvalid_tab_1_expr_idx
  parted_isvalid_tab_1_expr_idx  | f          | parted_isvalid_tab_1  | parted_isvalid_idx
  parted_isvalid_tab_2_expr_idx  | t          | parted_isvalid_tab_2  | parted_isvalid_idx
(5 rows)

The index  parted_isvalid_tab_11_expr_idx is created anew and it's valid,
but the higher-level indexes are still invalid.

> 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.

Yeah, in regard to that, your patch looks good to me.
Thank you!

Best regards,
Alexander