Skip to content

Backport: handle boolean settings better #6506

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.

Sign up for GitHub

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

Closed
systay wants to merge 3 commits into from
Closed

Backport: handle boolean settings better #6506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d657482
handle boolean settings better
systay Jul 30, 2020
4119405
removed boolean check from set extractvalue for colname
harshit-gangal Jul 30, 2020
de9ed4c
added endtoend test for on off settings
harshit-gangal Jul 30, 2020
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
26 changes: 21 additions & 5 deletions go/test/endtoend/vtgate/setstatement/sysvar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,41 @@ func TestSetSysVar(t *testing.T) {
}

queries := []queriesWithExpectations{{
name: "default_storage_engine",
name: "default_storage_engine", // ignored
expr: "INNODB",
expected: `[[VARCHAR("InnoDB")]]`,
}, {
name: "sql_mode",
name: "character_set_client", // check and ignored
expr: "utf8",
expected: `[[VARCHAR("utf8")]]`,
}, {
name: "character_set_client", // ignored so will keep the actual value
expr: "@charvar",
expected: `[[VARCHAR("utf8")]]`,
}, {
name: "sql_mode", // use reserved conn
expr: "''",
expected: `[[VARCHAR("")]]`,
}, {
name: "sql_mode",
name: "sql_mode", // use reserved conn
expr: `concat(@@sql_mode,"NO_ZERO_DATE")`,
expected: `[[VARCHAR("NO_ZERO_DATE")]]`,
}, {
name: "sql_mode",
name: "sql_mode", // use reserved conn
expr: "@@sql_mode",
expected: `[[VARCHAR("NO_ZERO_DATE")]]`,
}, {
name: "SQL_SAFE_UPDATES",
name: "SQL_SAFE_UPDATES", // use reserved conn
expr: "1",
expected: "[[INT64(1)]]",
}, {
name: "sql_auto_is_null", // ignored so will keep the actual value
expr: "on",
expected: `[[INT64(0)]]`,
}, {
name: "sql_notes", // use reserved conn
expr: "off",
expected: "[[INT64(0)]]",
}}

conn, err := mysql.Connect(ctx, &vtParams)
Expand Down
52 changes: 0 additions & 52 deletions go/vt/vtgate/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"sync"
"time"

"vitess.io/vitess/go/vt/log"
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"

"golang.org/x/net/context"
Expand Down Expand Up @@ -500,18 +499,6 @@ func handleSessionSetting(ctx context.Context, name string, session *SafeSession
default:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value for skip_query_plan_cache: %d", val)
}
case "sql_safe_updates":
val, err := validateSetOnOff(value, name)
if err != nil {
return err
}

switch val {
case 0, 1:
// no op
default:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value for sql_safe_updates: %d", val)
}
case "transaction_mode":
val, ok := value.(string)
if !ok {
Expand All @@ -522,17 +509,6 @@ func handleSessionSetting(ctx context.Context, name string, session *SafeSession
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid transaction_mode: %s", val)
}
session.TransactionMode = vtgatepb.TransactionMode(out)
case "tx_isolation": // TODO move this to set tx
val, ok := value.(string)
if !ok {
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value type for tx_isolation: %T", value)
}
switch val {
case "repeatable read", "read committed", "read uncommitted", "serializable":
// TODO (4127): This is a dangerous NOP.
default:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value for tx_isolation: %v", val)
}
case "tx_read_only", "transaction_read_only": // TODO move this to set tx
val, err := validateSetOnOff(value, name)
if err != nil {
Expand Down Expand Up @@ -575,34 +551,6 @@ func handleSessionSetting(ctx context.Context, name string, session *SafeSession
session.Options = &querypb.ExecuteOptions{}
}
session.Options.SqlSelectLimit = val
case "sql_auto_is_null":
val, ok := value.(int64)
if !ok {
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value type for sql_auto_is_null: %T", value)
}
switch val {
case 0:
// This is the default setting for MySQL. Do nothing.
case 1:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "sql_auto_is_null is not currently supported")
default:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value for sql_auto_is_null: %d", val)
}
case "character_set_results":
// This is a statement that mysql-connector-j sends at the beginning. We return a canned response for it.
switch value {
case nil, "binary", "utf8", "utf8mb4", "latin1":
default:
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "disallowed value for character_set_results: %v", value)
}
case "wait_timeout":
_, ok := value.(int64)
if !ok {
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unexpected value type for wait_timeout: %T", value)
}
case "sql_mode", "net_write_timeout", "net_read_timeout", "lc_messages", "collation_connection", "foreign_key_checks", "sql_quote_show_create", "unique_checks":
log.Warningf("Ignored inapplicable SET %v = %v", name, value)
warnings.Add("IgnoredSet", 1)
case "charset", "names":
val, ok := value.(string)
if !ok {
Expand Down
Loading