From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index
Date: Thu, 29 Jun 2023 12:00:00 +0300
Message-ID: <611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com> (raw)
In-Reply-To: <ZJva9997u5H0RQe1@paquier.xyz>
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>
Hi Michael,
28.06.2023 10:02, Michael Paquier wrote:
>> 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.
>
>
> I have looked into this one, and decided that we should just rely on
> the existing business in DefineIndex() where we switch indisvalid to
> false on a parent if the index found for the match is itself invalid.
> The code had two bugs:
> - First, we did not re-check if an index freshly created was itself
> invalid, which could happen when recursing through more than one
> level for the creation of a partitioned index.
> - We need to have a CCI after switch the parent's indisvalid to false,
> so as the follow up recursions are able to see that, and do the update
> of indisvalid across the parents up to the top-most parent.
>
> Attached is a patch to fix all that, with regression tests that play
> with multiple partition levels to show the recursion and the problems
> we'd have without the patch.
>
> Alexander, what do you think?
I'm somewhat confused by two things.
First, if we extend your test case by adding DETACH parted_isvalid_tab_11, we get:
indexrelid | indisvalid | indrelid | inhparent
--------------------------------+------------+-----------------------+-------------------------------
parted_isvalid_idx | f | parted_isvalid_tab |
parted_isvalid_idx_11 | f | parted_isvalid_tab_11 |
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)
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.
> 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.
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?
And even if so, which visible behavior this definition leads (or should
lead) to?
As I noted previously, the leaf index is still used when it's parent has
indisvalid = false. (That's the second thing that confuses me.)
For example, with the partition index tree:
indexrelid | indisvalid | indrelid | inhparent
--------------------------------+------------+-----------------------+-------------------------------
parted_isvalid_idx | f | parted_isvalid_tab |
parted_isvalid_idx_11 | f | parted_isvalid_tab_11 |
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
(6 rows)
explain select * from parted_isvalid_tab where a / b = 1;
Append (cost=0.12..23.14 rows=12 width=8)
-> Index Scan using parted_isvalid_tab_11_expr_idx on parted_isvalid_tab_11 parted_isvalid_tab_1 (cost=0.1
2..8.14 rows=1 width=8)
Index Cond: ((a / b) = 1)
-> Bitmap Heap Scan on parted_isvalid_tab_12 parted_isvalid_tab_2 (cost=4.24..14.94 rows=11 width=8)
Recheck Cond: ((a / b) = 1)
-> Bitmap Index Scan on parted_isvalid_tab_12_expr_idx (cost=0.00..4.24 rows=11 width=0)
Index Cond: ((a / b) = 1)
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.
Best regards,
Alexander
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index
In-Reply-To: <611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox