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

*: refactor ExecuteInternal to return single resultset (#22546) #22655

Merged

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Feb 2, 2021

cherry-pick #22546 to release-5.0-rc
You can switch your code base to this Pull Request by using git-extras:

# In tidb repo:
git pr https://github.com/pingcap/tidb/pull/22655

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/tidb.git pr/22655:release-5.0-rc-7ca1629d1a51

What problem does this PR solve?

Problem Summary:

Both Execute and ExecuteInternal no longer permit multi-statement, but the return type was maintained the same for backward compatibility.

This changes ExecuteInternal to return a single result-set.

Update: I've removed Execute from this PR, we can handle it in followup PRs.

What is changed and how it works?

What's Changed:

This changes the return type of ExecuteInternal to be a non-array value. All uses of Execute are changed to use ExecuteInternal (for future PRs - it does not need to be handled all at once).

Related changes

Check List

Tests

  • Covered by existing unit test

Side effects

  • More difficult cherry-picks of code

Release note

  • No release note

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot requested review from a team as code owners February 2, 2021 01:26
@ti-srebot ti-srebot requested review from XuHuaiyu and removed request for a team February 2, 2021 01:26
@ti-srebot
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@xhebox xhebox 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 Feb 4, 2021
@tiancaiamao
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 4, 2021
@tiancaiamao
Copy link
Contributor

/merge

1 similar comment
@morgo
Copy link
Contributor

morgo commented Feb 4, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 4, 2021
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 80a3b1d into pingcap:release-5.0-rc Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants