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

*: support where in admin show ddl jobs statement #12484

Merged
merged 12 commits into from Oct 11, 2019
Merged

*: support where in admin show ddl jobs statement #12484

merged 12 commits into from Oct 11, 2019

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Sep 29, 2019 •
edited by bb7133

What problem does this PR solve?

*: support where in admin show ddl statement

Related parser PR: pingcap/parser#568

eg:

mysql>admin show ddl jobs where job_type='create table'
+--------+---------+------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+-----------------------------------+--------+
| JOB_ID | DB_NAME | TABLE_NAME | JOB_TYPE     | SCHEMA_STATE | SCHEMA_ID | TABLE_ID | ROW_COUNT | START_TIME                        | END_TIME                          | STATE  |
+--------+---------+------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+-----------------------------------+--------+
| 136    | test    | t          | create table | public       | 1         | 135      | 0         | 2019-09-29 17:41:40.561 +0800 CST | 2019-09-29 17:41:40.565 +0800 CST | synced |
+--------+---------+------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+-----------------------------------+--------+

What is changed and how it works?

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Release note

  • SupportWHERE clause in ADMIN SHOW DDL JOBS statement

@crazycs520 crazycs520 changed the title *: support where in admin show ddl statement *: support where in admin show ddl jobs statement Sep 29, 2019
@sre-bot sre-bot mentioned this pull request Sep 30, 2019
Merged
@codecov
Copy link

codecov bot commented Oct 8, 2019 •
edited

Codecov Report

Merging #12484 into master will decrease coverage by 0.3605%.
The diff coverage is 80.6451%.

@@               Coverage Diff                @@
##             master     #12484        +/-   ##
================================================
- Coverage   80.2113%   79.8508%   -0.3606%     
================================================
  Files           461        460         -1     
  Lines        106167     103483      -2684     
================================================
- Hits          85158      82632      -2526     
+ Misses        14909      14781       -128     
+ Partials       6100       6070        -30

bb7133
bb7133 reviewed Oct 8, 2019
View reviewed changes
planner/core/planbuilder.go Outdated Show resolved Hide resolved
planner/core/planbuilder.go Outdated Show resolved Hide resolved
planner/core/initialize.go Outdated Show resolved Hide resolved
planner/core/initialize.go Outdated Show resolved Hide resolved
bb7133
bb7133 reviewed Oct 9, 2019
View reviewed changes
Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 9, 2019
lonng
lonng reviewed Oct 10, 2019
View reviewed changes
go.mod Outdated
@@ -78,3 +78,5 @@ require (
)

go 1.13

replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190929091254-85344bdd5ae3
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the parser PR have merged?

lonng
lonng reviewed Oct 10, 2019
View reviewed changes
executor/executor_test.go
@@ -456,7 +456,7 @@ func (s *testSuite) TestAdminShowDDLJobs(c *C) {
err = tk.Se.CommitTxn(context.Background())
c.Assert(err, IsNil)

re = tk.MustQuery("admin show ddl jobs 1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer to add some case to admin show ddl jobs 1 where ... instead of deleting the former case.

lonng
lonng reviewed Oct 11, 2019
View reviewed changes
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng lonng added status/LGT2 Indicates that a PR has LGTM 2. and removed status/DNM status/LGT1 Indicates that a PR has LGTM 1. labels Oct 11, 2019
@crazycs520
Copy link
Contributor Author

/run-all-tests

@crazycs520 crazycs520 added the status/can-merge Indicates a PR has been approved by a committer. label Oct 11, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 11, 2019

/run-all-tests

@crazycs520 crazycs520 merged commit ebc122b into pingcap:master Oct 11, 2019
@crazycs520 crazycs520 mentioned this pull request Feb 3, 2020
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bb7133 bb7133 bb7133 left review comments

@lonng lonng lonng approved these changes

Assignees
No one assigned
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/usability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@crazycs520 @sre-bot @bb7133 @lonng