feat: add fields for enhancing global index #297
Conversation
|
|
L-maple
force-pushed
the
feature/enhance_global_index
branch
from
26ff21b
to
abd3eac
Compare
|
/cc @mjonss |
| @@ -154,6 +154,8 @@ message IndexScan { | |||
| optional bool desc = 4 [(gogoproto.nullable) = false]; | |||
| optional bool unique = 5; // check whether it is a unique index. | |||
| repeated int64 primary_column_ids = 6; | |||
| optional bool is_global = 7; // check whether it is a global index. | |||
There was a problem hiding this comment.
Can you briefly explain why these two parameters are added?
And I think you should also modify its related logic on the TiKV side.
There was a problem hiding this comment.
is_global means whether it is a global index. Only when is_global is true, we should consider partition_ids. TiKV then filters out the rows by comparing partition_ids and PartitionID in index value.
For global index, Not all indexes' row should be returned when partitions are specified or some partitions are in reorg.
e.g., select * from pt partition(p0) use index(global_idx); The IndexScan DAG should contain p0 to filter the needed partitions' rows.
e.g., pt has p0,p1,p2 partitions. When p0 is in reorg, its index rows should not be queried.
In summary, necessary info should be contained in the IndexScan, so that TiKV can handle these circumstances correctly.
There was a problem hiding this comment.
OK, I will propose the PR for TiKV, and mention the PR here.
But please review this PR first, so that this tipb PR can be used in TiDB PR. Then the TiDB PR can be tested.
There was a problem hiding this comment.
Is it possible to use a Selection instead of combining an Index Scan with a filter/selection? I.e. do we need to add this PR?
There was a problem hiding this comment.
@mjonss Aha, it's a really nice idea. But it is really difficult for me in two aspects:
- What conditions I should put into Selection. Because ExtraPartitionID is not a real column.
- When I should construct the Selection.
There was a problem hiding this comment.
I'm OK in doing the filtering in TiDB for now and postpone the optimization of pushing down the filtering to a future PR. I just don't think we should change tipb repo like this, since I think the current protocol already have a more generic way of solving this.
There was a problem hiding this comment.
@mjonss @Defined2014 OK, I agree to postpone the optimization.
But I have to say the disadvantages：
- the code is really ugly in some places if partitionIds not pushed down.
- @mjonss How should I handle this case: [IndexScan(cop) <- HashAgg(cop) <- IndexReader(root)]. If
partitionIdsnot pushed down, Agg also not allowed pushed down. Otherwise, the result is wrong.
There was a problem hiding this comment.
@mjonss I remove the push down logic in TiDB PR, but remain related unit tests. This optimization(or fix) can be realized in future.
There was a problem hiding this comment.
I aggree that ExtraPartitionID should be treated as a normal column, and I think it is not a big problem about when/how to construct the selection, you can always contruct the filter/selection when your current code is setting is_global_index and partition_ids. There might be one benifit by putting is_global_index/partition_ids inside index scan: the index scan itself can skip some blocks based on these information. But for a well designed compiler, it should support push down filter to scan(not sure if TiKV support this)
There was a problem hiding this comment.
@windtalker OK, I will consider your advice and propose a new PR in future.
This PR can be closed.
|
/close |
2 similar comments
|
/close |
|
/close |
|
/close not-planned |
|
/assign @L-maple |
|
/close |
|
/close not-planned |
|
@Defined2014 Hi, how could I close this PR? |
|
Do you have permission to press the close pull button? |
|
/close |
Oh, I find it! Thanks~ |
What problem does this PR solve?
Issue Number: ref pingcap/tidb#41991
Issue Number: ref pingcap/tidb#42065
related PR: pingcap/tidb#41992
Problem Summary:
What is changed and how it works?
Proposal: global index
What's Changed:
How it Works:
Related changes
pingcap/docs/pingcap/docs-cn: