planner: not push down mpp/join to TiFlash in some cases that TiFlash not supported the query by windtalker · Pull Request #23259 · pingcap/tidb

windtalker · 2021-03-11T08:02:36Z

What problem does this PR solve?

In some cases, TiFlash does not support MPP execution, so TiDB should disable generating mpp plan.
Issue Number: close pingcap/tiflash#1520

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression
- Consumes more CPU
- Consumes more MEM
Breaking backward compatibility

Release note

Disable mpp execution if the new collation is on.

sre-bot · 2021-03-11T08:02:50Z

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what's changed

Or if the count of mainly changed packages are more than 3, use

*: what's changed

…hdown_to_tiflash_in_some_cases

hanfei1991 · 2021-03-11T08:25:01Z

/lgtm

…hdown_to_tiflash_in_some_cases

ichn-hu · 2021-03-11T08:51:48Z

/run-all-tests

…hdown_to_tiflash_in_some_cases

fzhedu · 2021-03-12T09:27:44Z

planner/core/integration_test.go

+	tk.MustExec("analyze table table_1")
+
+	// Create virtual tiflash replica info.
+	dom := domain.GetDomain(tk.Se)


would here set tiflash replica like this way?
https://github.com/pingcap/tidb/pull/23056/files#diff-71a2dde4dc9f29476f96c621ad0c11f6fc3f7b45185e11cb16f44acf46954cfeR328

fzhedu · 2021-03-12T09:33:14Z

planner/core/exhaust_physical_plans.go

 	return joins, true
 }

+func canExprsInJoinPushdown(p *LogicalJoin, storeType kv.StoreType) bool {


Are there similar case for agg?

…hdown_to_tiflash_in_some_cases

fzhedu · 2021-03-12T10:23:27Z

/LGTM

ti-chi-bot · 2021-03-12T10:23:30Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

fzhedu
hanfei1991

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

fzhedu · 2021-03-12T10:23:50Z

/merge

ti-chi-bot · 2021-03-12T10:23:53Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: c661abe

windtalker · 2021-03-12T10:25:45Z

/run-all-tests

windtalker · 2021-03-12T10:38:40Z

/run-check_dev_2

windtalker · 2021-03-12T10:53:49Z

/run-all-tests

windtalker · 2021-03-12T11:12:41Z

/run-check_dev_2

windtalker · 2021-03-12T12:07:06Z

/run-all-tests

This reverts commit c661abe.

windtalker · 2021-03-12T12:58:22Z

/run-all-tests

fzhedu · 2021-03-12T13:00:54Z

/lgtm

fzhedu · 2021-03-12T13:04:05Z

/merge

ti-chi-bot · 2021-03-12T13:04:07Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: ab4daa7

ti-chi-bot · 2021-03-12T13:19:12Z

@windtalker: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

windtalker added 3 commits March 11, 2021 15:55

disable mpp in some cases that tiflash is not supported

750ad8c

fix bug

dfe1c36

fix bug

d1ab637

windtalker requested review from a team as code owners March 11, 2021 08:02

windtalker requested review from XuHuaiyu and wshwsh12 and removed request for a team March 11, 2021 08:02

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 11, 2021

windtalker changed the title ~~Not pushdown to tiflash in some cases~~ planner: not push down mpp/join to TiFlash in some cases that TiFlash not supported the query Mar 11, 2021

Merge branch 'master' of https://github.com/pingcap/tidb into not_pus…

04ce3c4

…hdown_to_tiflash_in_some_cases

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 11, 2021

github-actions bot added the sig/execution SIG execution label Mar 11, 2021

Merge branch 'master' of https://github.com/pingcap/tidb into not_pus…

d19f240

…hdown_to_tiflash_in_some_cases

windtalker and others added 4 commits March 11, 2021 18:34

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

e1c1650

merge master

91fa561

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

910ff6a

Merge branch 'master' of https://github.com/pingcap/tidb into not_pus…

7999359

…hdown_to_tiflash_in_some_cases

fzhedu reviewed Mar 12, 2021

View reviewed changes

windtalker added 2 commits March 12, 2021 17:58

Merge branch 'master' of https://github.com/pingcap/tidb into not_pus…

3f15bcc

…hdown_to_tiflash_in_some_cases

address comments

c661abe

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 12, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 12, 2021

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

c6e6041

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

a8c4aa5

ti-chi-bot and others added 2 commits March 12, 2021 19:29

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

5691d88

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

2ecaf80

Revert "address comments"

ab4daa7

This reverts commit c661abe.

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 12, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 12, 2021

ti-chi-bot added 2 commits March 12, 2021 21:04

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

0da7984

Merge branch 'master' into not_pushdown_to_tiflash_in_some_cases

c6b5bff

ti-chi-bot merged commit cd199b2 into pingcap:master Mar 12, 2021

windtalker deleted the not_pushdown_to_tiflash_in_some_cases branch March 12, 2021 13:35

Conversation

windtalker commented Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

sre-bot commented Mar 11, 2021

Uh oh!

hanfei1991 commented Mar 11, 2021

Uh oh!

ichn-hu commented Mar 11, 2021

Uh oh!

fzhedu Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

fzhedu Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

windtalker Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

fzhedu commented Mar 12, 2021

Uh oh!

ti-chi-bot commented Mar 12, 2021

Uh oh!

fzhedu commented Mar 12, 2021

Uh oh!

ti-chi-bot commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

windtalker commented Mar 12, 2021

Uh oh!

fzhedu commented Mar 12, 2021

Uh oh!

fzhedu commented Mar 12, 2021

Uh oh!

ti-chi-bot commented Mar 12, 2021

Uh oh!

ti-chi-bot commented Mar 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

windtalker commented Mar 11, 2021 •

edited

Loading