check, diff: auto discover ansi-quotes #381
Conversation
lance6716
changed the title
|
PTAL @csuzhangxc @GMHDBJD |
| @@ -72,8 +72,6 @@ type DBConfig struct { | |||
| Schema string `toml:"schema" json:"schema"` | |||
|
|
|||
| Snapshot string `toml:"snapshot" json:"snapshot"` | |||
|
|
|||
| SQLMode string `toml:"sql-mode" json:"sql-mode"` | |||
There was a problem hiding this comment.
Do other places still need this (sql_mode may including others rather than ANSI_QUOTES)?
But we may remove it now and add it back if needed later.
There was a problem hiding this comment.
I guess other places don't require this SQLMode otherwise it won't compile. and in #280 this field is used for parser only
|
/run-all-tests |
|
/run-all-tests |
|
/run-integration-tests |
|
/run-all-tests |
|
very strange that integration test could pass on my Ubuntu 🤔 |
|
I could confirm integration test can pass on my Ubuntu and seems CI fails on PTAL @csuzhangxc @GMHDBJD |
|
/run-all-tests |
|
/run-integration-tests |
|
mysteriously CI pass, PTAL @csuzhangxc @GMHDBJD |
pkg/dbutil/common.go
Outdated
|
|
||
| // GetSQLMode returns sql_mode. | ||
| func GetSQLMode(db *sql.DB) (tmysql.SQLMode, error) { | ||
| sqlMode, err := GetGlobalVariable(db, "sql_mode") |
There was a problem hiding this comment.
Should be session variable because now we support session config for db？
pkg/check/table_structure.go
Outdated
| ansiQuotes, err := dbutil.HasAnsiQuotesMode(c.db) | ||
| if err != nil { | ||
| return []*incompatibilityOption{ | ||
| { | ||
| state: StateFailure, | ||
| errMessage: err.Error(), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| sqlMode := "" | ||
| if c.enableANSIQuotes { | ||
| if ansiQuotes { | ||
| sqlMode = "ANSI_QUOTES" | ||
| } | ||
| parser2, err := dbutil.GetParser(sqlMode) |
There was a problem hiding this comment.
How about combine them into one function?
pkg/dbutil/common.go
Outdated
| if ansiQuotes { | ||
| sqlMode = "ANSI_QUOTES" | ||
| } | ||
| return getParser(sqlMode) |
There was a problem hiding this comment.
Can we set sql_mode for all modes from GetSQLMode rather than only ANSI_QUOTES?
There was a problem hiding this comment.
Okay, going to align DM's parser to this behaviour
What problem does this PR solve?
auto discover ansi-quotes so don't need to set it in config, part of pingcap/dm#878
What is changed and how it works?
for processing
db(new connection) related query results, get ansi-quotes from global variable for this db.(may need to get session variable for connection related query results, not found yet)
Check List
Tests
Code changes
Side effects
Related changes