Skip to content

Commit

Permalink
less warnings when not applying system settings
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Taylor <[email protected]>
  • Loading branch information
systay committed Jul 24, 2020
1 parent acad359 commit a80e7f1
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 55 deletions.
56 changes: 22 additions & 34 deletions go/test/endtoend/vtgate/setstatement/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,53 +55,41 @@ func TestSetSysVar(t *testing.T) {
Port: clusterInstance.VtgateMySQLPort,
}
type queriesWithExpectations struct {
query string
expectedRows string
rowsAffected int
errMsg string
expectedWarning string
name, expr, expected string
}

queries := []queriesWithExpectations{{
query: `set @@default_storage_engine = INNODB`,
expectedRows: ``, rowsAffected: 0,
expectedWarning: "[[VARCHAR(\"Warning\") UINT16(1235) VARCHAR(\"Ignored inapplicable SET default_storage_engine = INNODB\")]]",
name: "default_storage_engine",
expr: "INNODB",
expected: `[[VARCHAR("InnoDB")]]`,
}, {
query: `set @@sql_mode = @@sql_mode`,
expectedRows: ``, rowsAffected: 0,
name: "sql_mode",
expr: "''",
expected: `[[VARCHAR("")]]`,
}, {
query: `set @@sql_mode = concat(@@sql_mode,"")`,
expectedRows: ``, rowsAffected: 0,
name: "sql_mode",
expr: `concat(@@sql_mode,"NO_ZERO_DATE")`,
expected: `[[VARCHAR("NO_ZERO_DATE")]]`,
}, {
query: `set @@SQL_SAFE_UPDATES = 1`,
expectedRows: ``, rowsAffected: 0,
name: "sql_mode",
expr: "@@sql_mode",
expected: `[[VARCHAR("NO_ZERO_DATE")]]`,
}, {
name: "SQL_SAFE_UPDATES",
expr: "1",
expected: "[[INT64(1)]]",
}}

conn, err := mysql.Connect(ctx, &vtParams)
require.NoError(t, err)
defer conn.Close()

for i, q := range queries {
t.Run(fmt.Sprintf("%d-%s", i, q.query), func(t *testing.T) {
qr, err := exec(t, conn, q.query)
if q.errMsg != "" {
require.Contains(t, err.Error(), q.errMsg)
} else {
require.NoError(t, err)
require.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query)
if q.expectedRows != "" {
result := fmt.Sprintf("%v", qr.Rows)
if diff := cmp.Diff(q.expectedRows, result); diff != "" {
t.Errorf("%s\nfor query: %s", diff, q.query)
}
}
if q.expectedWarning != "" {
qr := checkedExec(t, conn, "show warnings")
if got, want := fmt.Sprintf("%v", qr.Rows), q.expectedWarning; got != want {
t.Errorf("select:\n%v want\n%v", got, want)
}
}
}
query := fmt.Sprintf("set %s = %s", q.name, q.expr)
t.Run(fmt.Sprintf("%d-%s", i, query), func(t *testing.T) {
_, err := exec(t, conn, query)
require.NoError(t, err)
assertMatches(t, conn, fmt.Sprintf("select @@%s", q.name), q.expected)
})
}
}
Expand Down
19 changes: 14 additions & 5 deletions go/vt/vtgate/engine/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"encoding/json"
"fmt"

"vitess.io/vitess/go/vt/log"

"vitess.io/vitess/go/vt/srvtopo"

"vitess.io/vitess/go/vt/vtgate/evalengine"

"vitess.io/vitess/go/mysql"

"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/key"
querypb "vitess.io/vitess/go/vt/proto/query"
Expand Down Expand Up @@ -77,6 +77,15 @@ type (
TargetDestination key.Destination `json:",omitempty"`
Expr string
}

// SysVarSetSpecial implements the SetOp interface and will write the changes variable into the session
// The special part is that these settings change the sessions behaviour in different ways
SysVarSetSpecial struct {
Name string
Keyspace *vindexes.Keyspace
TargetDestination key.Destination `json:",omitempty"`
Expr string
}
)

var _ Primitive = (*Set)(nil)
Expand Down Expand Up @@ -197,8 +206,8 @@ func (svi *SysVarIgnore) VariableName() string {
}

//Execute implements the SetOp interface method.
func (svi *SysVarIgnore) Execute(vcursor VCursor, _ evalengine.ExpressionEnv) error {
vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr)})
func (svi *SysVarIgnore) Execute(VCursor, evalengine.ExpressionEnv) error {
log.Infof("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr)
return nil
}

Expand Down Expand Up @@ -237,7 +246,7 @@ func (svci *SysVarCheckAndIgnore) Execute(vcursor VCursor, env evalengine.Expres
return err
}
if result.RowsAffected == 0 {
vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Modification not allowed using set construct for: %s", svci.Name)})
log.Infof("Ignored inapplicable SET %v = %v", svci.Name, svci.Expr)
}
return nil
}
Expand Down
9 changes: 0 additions & 9 deletions go/vt/vtgate/engine/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ func TestSetTable(t *testing.T) {
Expr: "42",
},
},
expectedWarning: []*querypb.QueryWarning{
{Code: 1235, Message: "Ignored inapplicable SET x = 42"},
},
},
{
testName: "sysvar check and ignore",
Expand Down Expand Up @@ -152,9 +149,6 @@ func TestSetTable(t *testing.T) {
`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
`ExecuteMultiShard ks.-20: select 1 from dual where @@x = dummy_expr {} false false`,
},
expectedWarning: []*querypb.QueryWarning{
{Code: 1235, Message: "Modification not allowed using set construct for: x"},
},
},
{
testName: "sysvar checkAndIgnore multi destination error",
Expand Down Expand Up @@ -200,9 +194,6 @@ func TestSetTable(t *testing.T) {
`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
`ExecuteMultiShard ks.-20: select 1 from dual where @@z = dummy_expr {} false false`,
},
expectedWarning: []*querypb.QueryWarning{
{Code: 1235, Message: "Ignored inapplicable SET y = 2"},
},
qr: []*sqltypes.Result{sqltypes.MakeTestResult(
sqltypes.MakeTestFields(
"id",
Expand Down
7 changes: 1 addition & 6 deletions go/vt/vtgate/executor_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package vtgate
import (
"testing"

"vitess.io/vitess/go/mysql"
querypb "vitess.io/vitess/go/vt/proto/query"

"vitess.io/vitess/go/test/utils"
Expand Down Expand Up @@ -239,7 +238,7 @@ func TestExecutorSet(t *testing.T) {
out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}},
}, {
in: "set sql_auto_is_null = 0",
out: &vtgatepb.Session{Autocommit: true},
out: &vtgatepb.Session{Autocommit: true, RowCount: -1},
}, {
in: "set sql_auto_is_null = 1",
out: &vtgatepb.Session{Autocommit: true, RowCount: -1},
Expand Down Expand Up @@ -313,10 +312,6 @@ func TestExecutorSetOp(t *testing.T) {
sysVars map[string]string
}{{
in: "set big_tables = 1",
warning: []*querypb.QueryWarning{{
Code: mysql.ERNotSupportedYet,
Message: "Ignored inapplicable SET big_tables = 1",
}},
}, {
in: "set sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_UPDATES'",
sysVars: map[string]string{"sql_mode": "'STRICT_ALL_TABLES,NO_AUTO_UPDATES'"},
Expand Down
3 changes: 2 additions & 1 deletion go/vt/vtgate/planbuilder/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ var ignoreThese = []string{
"query_prealloc_size",
"sql_buffer_result",
"transaction_alloc_block_size",
"wait_timeout",
}

var saveSettingsToSession = []string{
Expand Down Expand Up @@ -138,7 +139,7 @@ var vitessShouldBeAwareOf = []string{
"time_zone",
"transaction_isolation",
"version_tokens_session",
"wait_timeout",
"sql_auto_is_null",
}

func init() {
Expand Down

0 comments on commit a80e7f1

Please sign in to comment.