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, bindinfo: support show bindings order by update_time #26139

Merged
merged 23 commits into from
Aug 4, 2021
Merged

planner, bindinfo: support show bindings order by update_time #26139

merged 23 commits into from
Aug 4, 2021

Conversation

Reminiscent
Copy link
Contributor

@Reminiscent Reminiscent commented Jul 12, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: close issue#15702

Problem Summary:
Support the show bindings order by update_time.

What is changed and how it works?

When we get the binding record from the cache, we show them order by update_time.

Check List

Tests

  • Unit test

Documentation

  • Affects user behaviors

Release note

  • support the show bindings order by update_time

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jul 12, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • eurekaka
  • winoros

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.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 12, 2021
@Reminiscent
Copy link
Contributor Author

/cc @eurekaka

@ti-chi-bot ti-chi-bot requested a review from eurekaka July 12, 2021 08:55
@github-actions github-actions bot added the sig/execution SIG execution label Jul 12, 2021
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 12, 2021
@github-actions github-actions bot added the sig/planner SIG: Planner label Jul 12, 2021
@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 14, 2021
@Reminiscent Reminiscent changed the title planner, bindinfo: support the show global bindings order by update_time planner, bindinfo: support show global bindings order by update_time Jul 15, 2021
@Reminiscent
Copy link
Contributor Author

/label epic/sql-plan-management

@ti-chi-bot
Copy link
Member

@Reminiscent: The label(s) epic/sql-plan-management cannot be applied. These labels are supported: CHANGELOG, challenge-program, compatibility-breaker, contribution, correctness, first-time-contributor, good-first-issue, hacktoberfest, high-performance, hptc, need-more-info, needs-cherry-pick-3.0, needs-cherry-pick-3.1, needs-cherry-pick-4.0, needs-cherry-pick-5.0, needs-cherry-pick-5.1, proposal, release-note, require-LGT3, security, tools.

In response to this:

/label epic/sql-plan-management

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.

eurekaka
eurekaka reviewed Aug 3, 2021
View reviewed changes
executor/show.go
// Remove the invalid bindRecord.
ind := 0
for _, bindData := range bindRecords {
if len(bindData.Bindings) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

How could this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the session binding is deleted, len(bindData.Bindings) maybe 0.

@Reminiscent Reminiscent changed the title planner, bindinfo: support show global bindings order by update_time planner, bindinfo: support show bindings order by update_time Aug 3, 2021
@ti-chi-bot
Copy link
Member

@Reminiscent: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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 kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Aug 3, 2021
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 3, 2021
@Reminiscent
Copy link
Contributor Author

Reminiscent commented Aug 4, 2021 •
edited
Loading

Update. PTAL @eurekaka @winoros

@zhouqiang-cl zhouqiang-cl removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Aug 4, 2021
@Reminiscent Reminiscent mentioned this pull request Aug 4, 2021
Merged
12 tasks
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 4, 2021
@ti-chi-bot 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 Aug 4, 2021
@Reminiscent
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 897ffe7

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 4, 2021
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2021
@ti-chi-bot ti-chi-bot removed status/can-merge Indicates a PR has been approved by a committer. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 4, 2021
@Reminiscent
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 7b25c17

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 4, 2021
@ti-chi-bot ti-chi-bot merged commit a0d0b48 into pingcap:master Aug 4, 2021
@Reminiscent Reminiscent deleted the issue#15702 branch August 5, 2021 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winoros winoros winoros approved these changes

@eurekaka eurekaka eurekaka approved these changes

Assignees
No one assigned
Labels
epic/sql-plan-management sig/execution SIG execution sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

result of show [global|session] bindings is unordered
5 participants
@Reminiscent @ti-chi-bot @eurekaka @winoros @zhouqiang-cl