feat: replace mysql keywords/reserved words #14
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hyj1991
reviewed
May 6, 2022
app/service/mysql.js
Outdated
| @@ -91,14 +91,15 @@ class MysqlService extends Service { | |||
|
|
|||
| /* table <apps> */ | |||
| getAppByAppId(appId) { | |||
| const sql = 'SELECT * FROM apps WHERE id = ?'; | |||
| const sql = 'SELECT *, app_name as name, app_owner as owner FROM apps WHERE id = ?'; | |||
| const params = [appId]; | |||
There was a problem hiding this comment.
这些改动没有必要，表格就是 apps，app_ 的前缀属于冗余信息。
There was a problem hiding this comment.
oh, 这个PR的目的其实是因为name / owner 这些属于mysql的 reserved words， 有很多DBA会要求开发人员重命名的。
There was a problem hiding this comment.
这个我持保留意见，Mysql 的保留字包含太多常见的使用名了，尤其是 Account 这些名词是重灾区，关于列名的重新设计我得再思考下，就当下而言没有遇到这块的太强烈的需求
There was a problem hiding this comment.
你可以单独留下列属性的修改，譬如 DOUBLE -> DECIMAL，这些确实很有用
hyj1991
reviewed
May 6, 2022
app/service/mysql.js
Outdated
| const params = [appId]; | ||
| return this.consoleQuery(sql, params).then(data => data[0] || {}); | ||
| } | ||
|
|
||
| /* table <files> */ | ||
| updateFileStatusByAppAgentFile(appId, agentId, filePath) { | ||
| const sql = 'UPDATE files SET status = ? WHERE app = ? AND agent = ? AND file = ? AND status = ?'; | ||
| const sql = | ||
| 'UPDATE files SET file_status = ? WHERE app = ? AND agent = ? AND file = ? AND file_status = ?'; | ||
| const params = [1, appId, agentId, filePath, 0]; |
hyj1991
reviewed
May 6, 2022
| @@ -154,7 +154,7 @@ CREATE TABLE `osinfo`( | |||
| `http_response_sent` INT UNSIGNED COMMENT 'http responeses sent in last 1 min', | |||
| `http_request_timeout` INT UNSIGNED COMMENT 'timeout http requests in last 1 min', | |||
| `http_patch_timeout` INT UNSIGNED COMMENT 'http patch timeout (s)', | |||
| `http_rt` DOUBLE COMMENT 'http average response time (ms)', | |||
| `http_rt` DECIMAL(5,2) COMMENT 'http average response time (ms)', | |||
|
|
|||
| `gm_modified` DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT 'system modify timestamp', | |||
| `gm_create` DATETIME DEFAULT CURRENT_TIMESTAMP COMMENT 'system create timestamp', | |||
There was a problem hiding this comment.
DATETIME 其实也可以改为更加通用的 timestamp 类型
There was a problem hiding this comment.
这个也不好说，我现在这家的dba要求我们用bigint 存
hyj1991
approved these changes
May 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is for issue X-Profiler/xprofiler-console#36
It's tested against my self-hosted xprofiler console.
Let me know if the changes are not proper.
Thanks