Skip to content
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: support partition pruning for IndexJoin inner table #19990

Merged
merged 8 commits into from
Sep 29, 2020

Conversation

imtbkcat
Copy link

@imtbkcat imtbkcat commented Sep 15, 2020

What problem does this PR solve?

Problem Summary: Although index join support partition table as inner table, it can not do partition prune according to the outer table look up keys. This pr fixes this problem.

What is changed and how it works?

What's Changed:

  • add ColumnOffset in partition expression informations, which is used for re-construct keys to locate partition.
  • add buildPartitionTableForInnerExecutor which will firstly prune partition according to query conditions and then re-construct partition locate key according to column offset and data from outer table.

How it Works:

When extract data from outer table, inner table builder will prune according to outer table data by extract partition keys and eval partition expression. If outer table data doesn't contain enough information, such as lack of partition keys, the pruning will fail.

Related changes

  • None

Check List

Tests

  • Unit test (will be added later)
  • Manual test

Side effects

  • None

Release note

  • Support partition pruning on IndexJoin when inner table is partition table.

@imtbkcat imtbkcat added type/enhancement The issue or PR belongs to an enhancement. component/executor labels Sep 15, 2020
@imtbkcat imtbkcat requested a review from a team as a code owner September 15, 2020 03:02
@imtbkcat imtbkcat requested review from XuHuaiyu and removed request for a team September 15, 2020 03:02
@github-actions github-actions bot added the sig/execution SIG execution label Sep 15, 2020
@imtbkcat
Copy link
Author

/run-common-test

@tiancaiamao
Copy link
Contributor

tiancaiamao commented Sep 16, 2020

This solution is not efficient enough, it just narrows down the partitions for a batch of data (lookUpContent in the code).

Before the change:

for each datum in lookUpContent
    DAGReq = calculate datum kv ranges
    for each partition in ALL partitions
    	send DAGReq to partition 

After the change:

for each datum in lookUpContent
    DAGReq = calculate datum kv ranges
    for each partition in [lookUpContent related] partitions
    	send DAGReq to partition 

The ideal case:

for each datum in lookUpContent
    DAGReq = calculate datum kv ranges
    send DAGReq to the exact partition this datum belongs to

@tiancaiamao
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 25, 2020
@tiancaiamao
Copy link
Contributor

In the current implementation, one table reader is bind to one table, so we have to construct a partition table and change its child -- table reader.
If we can support reading multiple tables data in one table reader, the code could be simplified a lot (especially true for this index join case).
I'll add support for one table reader reading multiple table data.
Before that being done, this PR is an improvement to the current situation.

@lysu
Copy link
Contributor

lysu commented Sep 28, 2020

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 28, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 28, 2020
@imtbkcat
Copy link
Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 29, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 132d981 into pingcap:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/executor sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants