-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
executor: handle index join for the new partition table implementation #18862
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18862 +/- ##
===========================================
Coverage 79.4730% 79.4730%
===========================================
Files 546 546
Lines 148220 148220
===========================================
Hits 117795 117795
Misses 20948 20948
Partials 9477 9477 |
executor/builder.go
Outdated
} | ||
|
||
nextPartition := nextPartitionForIndexLookUp{exec: ret} | ||
exec, err := buildPartitionTable(b, ts.Table, v.PruningConds, ret, nextPartition) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe have same question, when change test where p.id = t.id
to where p.c = t.id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit more difficult because the the reader reads chunk by chunk. Inside a chunk, each row may correspond to diffierent table.
The current TableReader/IndexReader/IndexLookUp have an assumption that the underlying table is fixed and not change, so the build key range logic need update in those three reader executor.
I plan to improve here later, not in this PR.
Although the partition pruning is not done for some case, the code should be able to run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/rebuild |
/rebuild rpc error: code = Unknown desc = can not get timestamp, may be not leader ("rpc error: code = Unknow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
/run-all-tests |
What problem does this PR solve?
Problem Summary:
In #18823, the three readers is translated to partition table.
But for index join, that is not sufficient, because then inner table is constructed dynamically during runtime.
What is changed and how it works?
What's Changed:
Handle partition table in
dataReaderBuilder
, build partition table for index join.How it Works:
In
buildExecutorForIndexJoinInternal
, there are some other cases, I'm not sure when the code run there:Related changes
Check List
Tests
Release note
This change is based on #18823, that PR should be merged first.