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

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

Merged
merged 12 commits into from
Oct 11, 2019

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Sep 29, 2019

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
@codecov
Copy link

codecov bot commented Oct 8, 2019

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

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
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
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?

@@ -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.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants