-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
*: fix 'db' and 'Info' column of 'show processlist' #10985
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10985 +/- ##
================================================
- Coverage 80.9364% 80.9317% -0.0047%
================================================
Files 418 418
Lines 89212 89227 +15
================================================
+ Hits 72205 72213 +8
- Misses 11777 11783 +6
- Partials 5230 5231 +1 |
Codecov Report
@@ Coverage Diff @@
## master #10985 +/- ##
===========================================
Coverage ? 80.9615%
===========================================
Files ? 418
Lines ? 89256
Branches ? 0
===========================================
Hits ? 72263
Misses ? 11756
Partials ? 5237 |
db = s.sessionVars.CurrentDB | ||
} | ||
|
||
var info interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about not changing the type of Info
from string
to interface{}
. We can do it this way:
info := "NULL"
if len(sql) > 0 {
info = sql
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind. Could you add a test like:
select * from information_schema.processlist where db is null;
select * from information_schema.processlist where info is null;
/run-all-tests |
infoschema/tables_test.go
Outdated
StmtCtx: tk.Se.GetSessionVars().StmtCtx, | ||
} | ||
tk.Se.SetSessionManager(sm) | ||
tk.MustQuery("select * from information_schema.PROCESSLIST order by ID;").Sort().Check( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've already used order by ID
in the sql, thus the .Sort()
is not needed.
/run-all-tests |
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@wshwsh12 please cherry pick this commit to release-2.1 and release-3.0 |
@@ -26,12 +26,12 @@ type ProcessInfo struct { | |||
ID uint64 | |||
User string | |||
Host string | |||
DB string | |||
DB interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use interface{}
here is a bad idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any better idea? @tiancaiamao
What problem does this PR solve?
#10903
The Info column of
show processlist
should be NULL if it is not executing any statement.The db column of
show processlist
should be NULL if it is not select one.What is changed and how it works?
open 2 mysql client and run
show processlist;
both.Before this pr:
This pr:
Check List
Tests
Code changes
Side effects
Related changes