Skip to content

*: refactor ExecuteInternal to return single resultset#22546

Merged
tiancaiamao merged 13 commits intopingcap:masterfrom
morgo:fix-execute-to-single-return
Feb 1, 2021
Merged

*: refactor ExecuteInternal to return single resultset#22546
tiancaiamao merged 13 commits intopingcap:masterfrom
morgo:fix-execute-to-single-return

Conversation

@morgo
Copy link
Contributor

@morgo morgo commented Jan 27, 2021

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

@morgo morgo requested review from a team as code owners January 27, 2021 02:55
@morgo morgo requested review from XuHuaiyu and removed request for a team January 27, 2021 02:55
@github-actions github-actions bot added sig/execution SIG execution sig/sql-infra SIG: SQL Infra sig/planner SIG: Planner labels Jan 27, 2021
@morgo
Copy link
Contributor Author

morgo commented Jan 27, 2021

Interesting. It looks like this breaks the API used by plugins. It might require some more thought on how/when to break it.

@morgo
Copy link
Contributor Author

morgo commented Jan 28, 2021

/run-build enterprise-plugin=pr/39

@morgo
Copy link
Contributor Author

morgo commented Jan 28, 2021

/run-all-tests enterprise-plugin=pr/39

@morgo morgo force-pushed the fix-execute-to-single-return branch from e943f72 to f9eea80 Compare January 28, 2021 19:13
@morgo morgo changed the title *: refactor Execute/ExecuteInternal usage for single-statement *: refactor ExecuteInternal and Execute Jan 28, 2021
@morgo
Copy link
Contributor Author

morgo commented Jan 28, 2021

/run-all-tests

@morgo
Copy link
Contributor Author

morgo commented Jan 28, 2021

/run-all-tests

@morgo
Copy link
Contributor Author

morgo commented Jan 28, 2021

/run-tics-test

@morgo morgo changed the title *: refactor ExecuteInternal and Execute *: refactor ExecuteInternal to return single resultset Jan 29, 2021
@morgo morgo requested a review from xhebox January 29, 2021 16:48
@morgo
Copy link
Contributor Author

morgo commented Jan 29, 2021

I've simplified this to only clean up ExecuteInternal. We can remove Execute in followup PRs.

PTAL @tiancaiamao @xhebox

@morgo morgo requested a review from AilinKid February 1, 2021 04:35
@tiancaiamao
Copy link
Contributor

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Feb 1, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 1, 2021
@ti-srebot ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Feb 1, 2021
@xhebox
Copy link
Contributor

xhebox commented Feb 1, 2021

/merge

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

/run-all-tests

@ti-srebot
Copy link
Contributor

@morgo merge failed.

@morgo
Copy link
Contributor Author

morgo commented Feb 1, 2021

/run-tics-test

@tiancaiamao tiancaiamao merged commit 7ca1629 into pingcap:master Feb 1, 2021
@morgo morgo deleted the fix-execute-to-single-return branch February 1, 2021 06:06
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Feb 1, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #22640

@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #22655

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Feb 2, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Feb 3, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #22712

ti-srebot added a commit that referenced this pull request Feb 4, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
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/LGT3 The PR has already had 3 LGTM. type/2.1-cherry-pick

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants