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

Allow STRICT_ALL_TABLES when enforcing strict_trans_tables #4534

Merged
merged 1 commit into from
Jan 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/ServerConfiguration.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ This rule is not strictly enforced. You are allowed to add these things, but at

Similar guidelines should be used when deciding to bypass Vitess to send statements directly to MySQL.

Vitess also requires you to turn on STRICT_TRANS_TABLES mode. Otherwise, it cannot accurately predict what will be written to the database.
Vitess also requires you to turn on STRICT_TRANS_TABLES or STRICT_ALL_TABLES mode. Otherwise, it cannot accurately predict what will be written to the database.

It’s safe to apply backward compatible DDLs directly to MySQL. VTTablets can be configured to periodically check the schema for changes.

Expand Down
9 changes: 5 additions & 4 deletions go/vt/vttablet/tabletserver/connpool/dbconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ var (
)

// VerifyMode is a helper method to verify mysql is running with
// sql_mode = STRICT_TRANS_TABLES and autocommit=ON. It also returns
// the current binlog format.
// sql_mode = STRICT_TRANS_TABLES or STRICT_ALL_TABLES and autocommit=ON.
// It also returns the current binlog format.
func (dbc *DBConn) VerifyMode(strictTransTables bool) (BinlogFormat, error) {
if strictTransTables {
qr, err := dbc.conn.ExecuteFetch(getModeSQL, 2, false)
Expand All @@ -242,8 +242,9 @@ func (dbc *DBConn) VerifyMode(strictTransTables bool) (BinlogFormat, error) {
if len(qr.Rows) != 1 {
return 0, fmt.Errorf("incorrect rowcount received for %s: %d", getModeSQL, len(qr.Rows))
}
if !strings.Contains(qr.Rows[0][0].ToString(), "STRICT_TRANS_TABLES") {
return 0, fmt.Errorf("require sql_mode to be STRICT_TRANS_TABLES: got '%s'", qr.Rows[0][0].ToString())
sqlMode := qr.Rows[0][0].ToString()
if !(strings.Contains(sqlMode, "STRICT_TRANS_TABLES") || strings.Contains(sqlMode, "STRICT_ALL_TABLES")) {
return 0, fmt.Errorf("require sql_mode to be STRICT_TRANS_TABLES or STRICT_ALL_TABLES: got '%s'", qr.Rows[0][0].ToString())
}
}
qr, err := dbc.conn.ExecuteFetch(getAutocommit, 2, false)
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vttablet/tabletserver/query_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (
querypb "vitess.io/vitess/go/vt/proto/query"
)

func TestStrictTransTables(t *testing.T) {
func TestStrictMode(t *testing.T) {
db := fakesqldb.New(t)
defer db.Close()
for query, result := range schematest.Queries() {
Expand All @@ -61,7 +61,7 @@ func TestStrictTransTables(t *testing.T) {
}
qe.Close()

// Check that we fail if STRICT_TRANS_TABLES is not set.
// Check that we fail if STRICT_TRANS_TABLES or STRICT_ALL_TABLES is not set.
db.AddQuery(
"select @@global.sql_mode",
&sqltypes.Result{
Expand All @@ -72,7 +72,7 @@ func TestStrictTransTables(t *testing.T) {
qe = NewQueryEngine(DummyChecker, schema.NewEngine(DummyChecker, config), config)
qe.InitDBConfig(dbcfgs)
err := qe.Open()
wantErr := "require sql_mode to be STRICT_TRANS_TABLES: got ''"
wantErr := "require sql_mode to be STRICT_TRANS_TABLES or STRICT_ALL_TABLES: got ''"
if err == nil || err.Error() != wantErr {
t.Errorf("Open: %v, want %s", err, wantErr)
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/tabletenv/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func init() {
flag.BoolVar(&Config.HeartbeatEnable, "heartbeat_enable", DefaultQsConfig.HeartbeatEnable, "If true, vttablet records (if master) or checks (if replica) the current time of a replication heartbeat in the table _vt.heartbeat. The result is used to inform the serving state of the vttablet via healthchecks.")
flag.DurationVar(&Config.HeartbeatInterval, "heartbeat_interval", DefaultQsConfig.HeartbeatInterval, "How frequently to read and write replication heartbeat.")

flag.BoolVar(&Config.EnforceStrictTransTables, "enforce_strict_trans_tables", DefaultQsConfig.EnforceStrictTransTables, "If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database.")
flag.BoolVar(&Config.EnforceStrictTransTables, "enforce_strict_trans_tables", DefaultQsConfig.EnforceStrictTransTables, "If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES or STRICT_ALL_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database.")
flag.BoolVar(&Config.EnableConsolidator, "enable-consolidator", DefaultQsConfig.EnableConsolidator, "This option enables the query consolidator.")
}

Expand Down