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.

Sign up for GitHub

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

Jump to bottom

planner: copy the field Column in dataSource when create partition union. #20559

Merged
merged 4 commits into from
Oct 21, 2020
Merged

planner: copy the field Column in dataSource when create partition union. #20559

merged 4 commits into from
Oct 21, 2020

Conversation

wshwsh12
Copy link
Contributor

@wshwsh12 wshwsh12 commented Oct 21, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number: close #20558

Problem Summary: panic/wrong result when using partition union and prune columns optimize.

  1. Create datasouce when build partition union.
    Eg, datasource1, datasource2. They have the same address for field columns [column1,column2,column3,column4]
  2. Use prune columns optimization for each datasource. In prune columns optimization, it use schema(not columns, because it assume the schema is same to columns.) to check whether the columns is used.
    Assume column2 is useless.
    1. prune columns for datasource1.
      The columns in datasource1 has changed to [column1, column3, column4]
      The columns in datasource2 has changed to [column1, column3, column4, column4]
    2. prune columns for datasource2.
      The columns in datasoucres1 has changed to [column1, column4, column4]
      The columns in datasoucres1 has changed to [column1, column4, column4]
  3. Wrong columns info will build a wrong request and wrong result. After geting data from TiKV, it get panic when decoding.

What is changed and how it works?

Proposal: xxx

What's Changed:
copy the field Column in dataSource when create partition union.

How it Works:

Related changes

  • Need to cherry-pick to the release branch 4.0

Check List

Tests

  • Unit test
  • Integration test

Side effects

Release note

  • Fix unexpected panic when using partition tables.

@wshwsh12 wshwsh12 requested a review from a team as a code owner October 21, 2020 06:40
@wshwsh12 wshwsh12 requested review from SunRunAway and removed request for a team October 21, 2020 06:40
@wshwsh12 wshwsh12 added type/bugfix This PR fixes a bug. sig/planner SIG: Planner needs-cherry-pick-4.0 labels Oct 21, 2020
lzmhhh123
lzmhhh123 reviewed Oct 21, 2020
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 21, 2020
XuHuaiyu
XuHuaiyu reviewed Oct 21, 2020
View reviewed changes
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 21, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 21, 2020
@wshwsh12
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

@wshwsh12 Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: planner(slack).

@lzmhhh123
Copy link
Contributor

/merge

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

/run-all-tests

@ti-srebot
Copy link
Contributor

@wshwsh12 merge failed.

@wshwsh12
Copy link
Contributor Author

/run-unit-test

@wshwsh12
Copy link
Contributor Author

/run-all-tests

@wshwsh12
Copy link
Contributor Author

/run-unit-test

@wshwsh12
Copy link
Contributor Author

/run-sqllogic-test

@wshwsh12 wshwsh12 merged commit c2d94ad into pingcap:master Oct 21, 2020
@ti-srebot ti-srebot mentioned this pull request Oct 21, 2020
Merged
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Oct 21, 2020
@wshwsh12 @ti-srebot
cherry pick pingcap#20559 to release-4.0
bc0a12e 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #20565

wshwsh12 added a commit to wshwsh12/tidb that referenced this pull request Oct 21, 2020
@wshwsh12
planner: copy the field Column in dataSource when create partition un…
c6b4095 
…ion. (pingcap#20559)
ti-srebot added a commit that referenced this pull request Oct 22, 2020
@ti-srebot
planner: copy the field Column in dataSource when create partition un…
a0515bc 
…ion. (#20559) (#20565)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@wshwsh12 wshwsh12 deleted the prune_column_fix branch January 29, 2022 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuHuaiyu XuHuaiyu XuHuaiyu left review comments

@lzmhhh123 lzmhhh123 lzmhhh123 left review comments

@ti-srebot ti-srebot ti-srebot approved these changes

@SunRunAway SunRunAway Awaiting requested review from SunRunAway

Assignees
No one assigned
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Panic/wrong reuslt when using partition table.
4 participants
@wshwsh12 @ti-srebot @lzmhhh123 @XuHuaiyu