From: | Alexander Lakhin <exclusion(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
Subject: | Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index |
Date: | 2023-06-29 09:00:00 |
Message-ID: | 611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
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
From | Date | Subject | |
---|---|---|---|
Next Message | torikoshia | 2023-06-29 09:42:37 | Re: pg_rewind WAL segments deletion pitfall |
Previous Message | PG Bug reporting form | 2023-06-29 08:04:21 | BUG #18006: recovery_target_action=shutdown triggers automatic recovery on next startup (beyond point in time) |