Repository: vitessio/vitess
Pull request #6069 (2020-04-15 04:50:39)

Planning Set Statement for System Variable with Ignore and CheckAndIgnore


Patch:
diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go
index 3cecd6dd5a..72670d2d7a 100644
--- a/go/vt/vtgate/engine/primitive.go
+++ b/go/vt/vtgate/engine/primitive.go
@@ -42,61 +42,101 @@ const (
 	ListVarName = "__vals"
 )
 
-// VCursor defines the interface the engine will use
-// to execute routes.
-type VCursor interface {
-	// Context returns the context of the current request.
-	Context() context.Context
+type (
+	// VCursor defines the interface the engine will use
+	// to execute routes.
+	VCursor interface {
+		// Context returns the context of the current request.
+		Context() context.Context
 
-	// MaxMemoryRows returns the maxMemoryRows flag value.
-	MaxMemoryRows() int
+		// MaxMemoryRows returns the maxMemoryRows flag value.
+		MaxMemoryRows() int
 
-	// SetContextTimeout updates the context and sets a timeout.
-	SetContextTimeout(timeout time.Duration) context.CancelFunc
+		// SetContextTimeout updates the context and sets a timeout.
+		SetContextTimeout(timeout time.Duration) context.CancelFunc
 
-	// RecordWarning stores the given warning in the current session
-	RecordWarning(warning *querypb.QueryWarning)
+		// V3 functions.
+		Execute(method string, query string, bindvars map[string]*querypb.BindVariable, rollbackOnError bool, co vtgatepb.CommitOrder) (*sqltypes.Result, error)
+		AutocommitApproval() bool
 
-	// V3 functions.
-	Execute(method string, query string, bindvars map[string]*querypb.BindVariable, rollbackOnError bool, co vtgatepb.CommitOrder) (*sqltypes.Result, error)
-	AutocommitApproval() bool
+		// Shard-level functions.
+		ExecuteMultiShard(rss []*srvtopo.ResolvedShard, queries []*querypb.BoundQuery, rollbackOnError, canAutocommit bool) (*sqltypes.Result, []error)
+		ExecuteStandalone(query string, bindvars map[string]*querypb.BindVariable, rs *srvtopo.ResolvedShard) (*sqltypes.Result, error)
+		StreamExecuteMulti(query string, rss []*srvtopo.ResolvedShard, bindVars []map[string]*querypb.BindVariable, callback func(reply *sqltypes.Result) error) error
 
-	// Shard-level functions.
-	ExecuteMultiShard(rss []*srvtopo.ResolvedShard, queries []*querypb.BoundQuery, rollbackOnError, canAutocommit bool) (*sqltypes.Result, []error)
-	ExecuteStandalone(query string, bindvars map[string]*querypb.BindVariable, rs *srvtopo.ResolvedShard) (*sqltypes.Result, error)
-	StreamExecuteMulti(query string, rss []*srvtopo.ResolvedShard, bindVars []map[string]*querypb.BindVariable, callback func(reply *sqltypes.Result) error) error
+		// Keyspace ID level functions.
+		ExecuteKeyspaceID(keyspace string, ksid []byte, query string, bindVars map[string]*querypb.BindVariable, rollbackOnError, autocommit bool) (*sqltypes.Result, error)
 
-	// Keyspace ID level functions.
-	ExecuteKeyspaceID(keyspace string, ksid []byte, query string, bindVars map[string]*querypb.BindVariable, rollbackOnError, autocommit bool) (*sqltypes.Result, error)
+		// Resolver methods, from key.Destination to srvtopo.ResolvedShard.
+		// Will replace all of the Topo functions.
+		ResolveDestinations(keyspace string, ids []*querypb.Value, destinations []key.Destination) ([]*srvtopo.ResolvedShard, [][]*querypb.Value, error)
 
-	// Resolver methods, from key.Destination to srvtopo.ResolvedShard.
-	// Will replace all of the Topo functions.
-	ResolveDestinations(keyspace string, ids []*querypb.Value, destinations []key.Destination) ([]*srvtopo.ResolvedShard, [][]*querypb.Value, error)
+		ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error
 
-	SetTarget(target string) error
-	SetUDV(key string, value interface{}) error
+		Session() SessionActions
+	}
 
-	ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error
-}
+	//SessionActions gives primitives ability to interact with the session state
+	SessionActions interface {
+		// RecordWarning stores the given warning in the current session
+		RecordWarning(warning *querypb.QueryWarning)
 
-// Plan represents the execution strategy for a given query.
-// For now it's a simple wrapper around the real instructions.
-// An instruction (aka Primitive) is typically a tree where
-// each node does its part by combining the results of the
-// sub-nodes.
-type Plan struct {
-	Type                   sqlparser.StatementType // The type of query we have
-	Original               string                  // Original is the original query.
-	Instructions           Primitive               // Instructions contains the instructions needed to fulfil the query.
-	sqlparser.BindVarNeeds                         // Stores BindVars needed to be provided as part of expression rewriting
-
-	mu           sync.Mutex    // Mutex to protect the fields below
-	ExecCount    uint64        // Count of times this plan was executed
-	ExecTime     time.Duration // Total execution time
-	ShardQueries uint64        // Total number of shard queries
-	Rows         uint64        // Total number of rows
-	Errors       uint64        // Total number of errors
-}
+		SetTarget(target string) error
+		SetUDV(key string, value interface{}) error
+	}
+
+	// Plan represents the execution strategy for a given query.
+	// For now it's a simple wrapper around the real instructions.
+	// An instruction (aka Primitive) is typically a tree where
+	// each node does its part by combining the results of the
+	// sub-nodes.
+	Plan struct {
+		Type                   sqlparser.StatementType // The type of query we have
+		Original               string                  // Original is the original query.
+		Instructions           Primitive               // Instructions contains the instructions needed to fulfil the query.
+		sqlparser.BindVarNeeds                         // Stores BindVars needed to be provided as part of expression rewriting
+
+		mu           sync.Mutex    // Mutex to protect the fields below
+		ExecCount    uint64        // Count of times this plan was executed
+		ExecTime     time.Duration // Total execution time
+		ShardQueries uint64        // Total number of shard queries
+		Rows         uint64        // Total number of rows
+		Errors       uint64        // Total number of errors
+	}
+
+	// Match is used to check if a Primitive matches
+	Match func(node Primitive) bool
+
+	// Primitive is the building block of the engine execution plan. They form a tree structure, where the leaves typically
+	// issue queries to one or more vttablet.
+	// During execution, the Primitive's pass Result objects up the tree structure, until reaching the root,
+	// and its result is passed to the client.
+	Primitive interface {
+		RouteType() string
+		GetKeyspaceName() string
+		GetTableName() string
+		Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error)
+		StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error
+		GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error)
+		NeedsTransaction() bool
+
+		// The inputs to this Primitive
+		Inputs() []Primitive
+
+		// description is the description, sans the inputs, of this Primitive.
+		// to get the plan description with all children, use PrimitiveToPlanDescription()
+		description() PrimitiveDescription
+	}
+
+	// noInputs default implementation for Primitives that are leaves
+	noInputs struct{}
+
+	// noTxNeeded is a default implementation for Primitives that don't need transaction handling
+	noTxNeeded struct{}
+
+	// txNeeded is a default implementation for Primitives that need transaction handling
+	txNeeded struct{}
+)
 
 // AddStats updates the plan execution statistics
 func (p *Plan) AddStats(execCount uint64, execTime time.Duration, shardQueries, rows, errors uint64) {
@@ -121,9 +161,6 @@ func (p *Plan) Stats() (execCount uint64, execTime time.Duration, shardQueries,
 	return
 }
 
-// Match is used to check if a Primitive matches
-type Match func(node Primitive) bool
-
 // Find will return the first Primitive that matches the evaluate function. If no match is found, nil will be returned
 func Find(isMatch Match, start Primitive) Primitive {
 	if isMatch(start) {
@@ -180,42 +217,15 @@ func (p *Plan) MarshalJSON() ([]byte, error) {
 	return json.Marshal(marshalPlan)
 }
 
-// Primitive is the building block of the engine execution plan. They form a tree structure, where the leaves typically
-// issue queries to one or more vttablet.
-// During execution, the Primitive's pass Result objects up the tree structure, until reaching the root,
-// and its result is passed to the client.
-type Primitive interface {
-	RouteType() string
-	GetKeyspaceName() string
-	GetTableName() string
-	Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error)
-	StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error
-	GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error)
-	NeedsTransaction() bool
-
-	// The inputs to this Primitive
-	Inputs() []Primitive
-
-	// description is the description, sans the inputs, of this Primitive.
-	// to get the plan description with all children, use PrimitiveToPlanDescription()
-	description() PrimitiveDescription
-}
-
-type noInputs struct{}
-
 // Inputs implements no inputs
 func (noInputs) Inputs() []Primitive {
 	return nil
 }
 
-type noTxNeeded struct{}
-
 func (noTxNeeded) NeedsTransaction() bool {
 	return false
 }
 
-type txNeeded struct{}
-
 func (txNeeded) NeedsTransaction() bool {
 	return true
 }
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index 9fff1e5afe..6846ac5498 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -254,7 +254,7 @@ func (route *Route) execute(vcursor VCursor, bindVars map[string]*querypb.BindVa
 			for _, err := range errs {
 				if err != nil {
 					serr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError)
-					vcursor.RecordWarning(&querypb.QueryWarning{Code: uint32(serr.Num), Message: err.Error()})
+					vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: uint32(serr.Num), Message: err.Error()})
 				}
 			}
 			// fall through
diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
index f4e2b02560..42d5a173f2 100644
--- a/go/vt/vtgate/engine/set.go
+++ b/go/vt/vtgate/engine/set.go
@@ -17,8 +17,17 @@ limitations under the License.
 package engine
 
 import (
+	"encoding/json"
+	"fmt"
+
+	"vitess.io/vitess/go/mysql"
+
 	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/key"
 	querypb "vitess.io/vitess/go/vt/proto/query"
+	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
+	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
 type (
@@ -40,6 +49,20 @@ type (
 		Name      string
 		PlanValue sqltypes.PlanValue
 	}
+
+	// SysVarIgnore implements the SetOp interface to ignore the settings.
+	SysVarIgnore struct {
+		Name string
+		Expr string
+	}
+
+	// SysVarCheckAndIgnore implements the SetOp interface to check underlying setting and ignore if same.
+	SysVarCheckAndIgnore struct {
+		Name              string
+		Keyspace          *vindexes.Keyspace
+		TargetDestination key.Destination
+		CheckSysVarQuery  string
+	}
 )
 
 var _ Primitive = (*Set)(nil)
@@ -93,6 +116,18 @@ func (s *Set) description() PrimitiveDescription {
 
 var _ SetOp = (*UserDefinedVariable)(nil)
 
+//MarshalJSON provides the type to SetOp for plan json
+func (u *UserDefinedVariable) MarshalJSON() ([]byte, error) {
+	return json.Marshal(struct {
+		Type string
+		UserDefinedVariable
+	}{
+		Type:                "UserDefinedVariable",
+		UserDefinedVariable: *u,
+	})
+
+}
+
 //VariableName implements the SetOp interface method.
 func (u *UserDefinedVariable) VariableName() string {
 	return u.Name
@@ -104,5 +139,69 @@ func (u *UserDefinedVariable) Execute(vcursor VCursor, bindVars map[string]*quer
 	if err != nil {
 		return err
 	}
-	return vcursor.SetUDV(u.Name, value)
+	return vcursor.Session().SetUDV(u.Name, value)
+}
+
+var _ SetOp = (*SysVarIgnore)(nil)
+
+//MarshalJSON provides the type to SetOp for plan json
+func (svi *SysVarIgnore) MarshalJSON() ([]byte, error) {
+	return json.Marshal(struct {
+		Type string
+		SysVarIgnore
+	}{
+		Type:         "SysVarIgnore",
+		SysVarIgnore: *svi,
+	})
+
+}
+
+//VariableName implements the SetOp interface method.
+func (svi *SysVarIgnore) VariableName() string {
+	return svi.Name
+}
+
+//Execute implements the SetOp interface method.
+func (svi *SysVarIgnore) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error {
+	vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr)})
+	return nil
+}
+
+var _ SetOp = (*SysVarCheckAndIgnore)(nil)
+
+//MarshalJSON provides the type to SetOp for plan json
+func (svci *SysVarCheckAndIgnore) MarshalJSON() ([]byte, error) {
+	return json.Marshal(struct {
+		Type string
+		SysVarCheckAndIgnore
+	}{
+		Type:                 "SysVarCheckAndIgnore",
+		SysVarCheckAndIgnore: *svci,
+	})
+
+}
+
+//VariableName implements the SetOp interface method
+func (svci *SysVarCheckAndIgnore) VariableName() string {
+	return svci.Name
+}
+
+//Execute implements the SetOp interface method
+func (svci *SysVarCheckAndIgnore) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error {
+	rss, _, err := vcursor.ResolveDestinations(svci.Keyspace.Name, nil, []key.Destination{svci.TargetDestination})
+	if err != nil {
+		return vterrors.Wrap(err, "SysVarCheckAndIgnore")
+	}
+
+	if len(rss) != 1 {
+		return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Unexpected error, DestinationKeyspaceID mapping to multiple shards: %v", svci.TargetDestination)
+	}
+	result, err := execShard(vcursor, svci.CheckSysVarQuery, bindVars, rss[0], false /* rollbackOnError */, false /* canAutocommit */)
+	if err != nil {
+		return err
+	}
+	if result.RowsAffected != 1 {
+		return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Modification not allowed using set construct for: %s", svci.Name)
+	}
+	return nil
 }
diff --git a/go/vt/vtgate/engine/update_target.go b/go/vt/vtgate/engine/update_target.go
index f297c43e94..f637d4166e 100644
--- a/go/vt/vtgate/engine/update_target.go
+++ b/go/vt/vtgate/engine/update_target.go
@@ -60,7 +60,7 @@ func (updTarget UpdateTarget) GetTableName() string {
 
 // Execute implements the Primitive interface
 func (updTarget UpdateTarget) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, wantfields bool) (*sqltypes.Result, error) {
-	err := vcursor.SetTarget(updTarget.Target)
+	err := vcursor.Session().SetTarget(updTarget.Target)
 	if err != nil {
 		return nil, err
 	}
diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
index 4722a405fc..9330e9dd46 100644
--- a/go/vt/vtgate/planbuilder/set.go
+++ b/go/vt/vtgate/planbuilder/set.go
@@ -17,29 +17,89 @@ limitations under the License.
 package planbuilder
 
 import (
+	"fmt"
+
+	"vitess.io/vitess/go/vt/key"
+	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/sqlparser"
+	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 )
 
+var sysVarPlanningFunc = map[string]func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error){}
+
+func init() {
+	sysVarPlanningFunc["default_storage_engine"] = buildSetOpIgnore
+	sysVarPlanningFunc["sql_mode"] = buildSetOpCheckAndIgnore
+}
+
 func buildSetPlan(sql string, stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive, error) {
 	var setOps []engine.SetOp
+	var setOp engine.SetOp
+	var err error
+
+	if stmt.Scope == sqlparser.GlobalStr {
+		return nil, vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported in set: global")
+	}
 
 	for _, expr := range stmt.Exprs {
 		switch expr.Name.AtCount() {
 		case sqlparser.SingleAt:
+			pv, err := sqlparser.NewPlanValue(expr.Expr)
+			if err != nil {
+				return nil, err
+			}
+			setOp = &engine.UserDefinedVariable{
+				Name:      expr.Name.Lowered(),
+				PlanValue: pv,
+			}
+		case sqlparser.DoubleAt:
+			planFunc, ok := sysVarPlanningFunc[expr.Name.Lowered()]
+			if !ok {
+				return nil, ErrPlanNotSupported
+			}
+			setOp, err = planFunc(expr, vschema)
+			if err != nil {
+				return nil, err
+			}
 		default:
 			return nil, ErrPlanNotSupported
 		}
-		pv, err := sqlparser.NewPlanValue(expr.Expr)
-		if err != nil {
-			return nil, err
-		}
-		setOps = append(setOps, &engine.UserDefinedVariable{
-			Name:      expr.Name.Lowered(),
-			PlanValue: pv,
-		})
+		setOps = append(setOps, setOp)
 	}
 	return &engine.Set{
 		Ops: setOps,
 	}, nil
 }
+
+func buildSetOpIgnore(expr *sqlparser.SetExpr, _ ContextVSchema) (engine.SetOp, error) {
+	buf := sqlparser.NewTrackedBuffer(nil)
+	buf.Myprintf("%v", expr.Expr)
+
+	return &engine.SysVarIgnore{
+		Name: expr.Name.Lowered(),
+		Expr: buf.String(),
+	}, nil
+}
+
+func buildSetOpCheckAndIgnore(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
+	keyspace, err := vschema.DefaultKeyspace()
+	if err != nil {
+		return nil, err
+	}
+
+	dest := vschema.Destination()
+	if dest == nil {
+		dest = key.DestinationAnyShard{}
+	}
+
+	buf := sqlparser.NewTrackedBuffer(nil)
+	buf.Myprintf("%v", expr.Expr)
+
+	return &engine.SysVarCheckAndIgnore{
+		Name:              expr.Name.Lowered(),
+		Keyspace:          keyspace,
+		TargetDestination: dest,
+		CheckSysVarQuery:  fmt.Sprintf("select 1 from dual where @@%s = %s", expr.Name.Lowered(), buf.String()),
+	}, nil
+}
diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go
index 02a167846e..25ec1cb4ce 100644
--- a/go/vt/vtgate/vcursor_impl.go
+++ b/go/vt/vtgate/vcursor_impl.go
@@ -312,6 +312,10 @@ func (vc *vcursorImpl) ResolveDestinations(keyspace string, ids []*querypb.Value
 	return vc.resolver.ResolveDestinations(vc.ctx, keyspace, vc.tabletType, ids, destinations)
 }
 
+func (vc *vcursorImpl) Session() engine.SessionActions {
+	return vc
+}
+
 func (vc *vcursorImpl) SetTarget(target string) error {
 	keyspace, tabletType, _, err := parseDestinationTarget(target, vc.vschema)
 	if err != nil {


Test patch:
diff --git a/go/test/endtoend/vtgate/setstatement/main_test.go b/go/test/endtoend/vtgate/setstatement/main_test.go
new file mode 100644
index 0000000000..1bb654e694
--- /dev/null
+++ b/go/test/endtoend/vtgate/setstatement/main_test.go
@@ -0,0 +1,101 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package setstatement
+
+import (
+	"flag"
+	"os"
+	"testing"
+
+	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/test/endtoend/cluster"
+)
+
+var (
+	clusterInstance *cluster.LocalProcessCluster
+	keyspaceName    = "ks"
+	cell            = "zone1"
+	hostname        = "localhost"
+	sqlSchema       = `
+	create table test(
+		id bigint,
+		val1 varchar(16),
+		val2 int,
+		val3 float,
+		primary key(id)
+	)Engine=InnoDB;`
+
+	vSchema = `
+		{	
+			"sharded":true,
+			"vindexes": {
+				"hash_index": {
+					"type": "hash"
+				}
+			},	
+			"tables": {
+				"test":{
+					"column_vindexes": [
+						{
+							"column": "id",
+							"name": "hash_index"
+						}
+					]
+				}
+			}
+		}
+	`
+)
+
+func TestMain(m *testing.M) {
+	defer cluster.PanicHandler(nil)
+	flag.Parse()
+
+	exitCode := func() int {
+		clusterInstance = cluster.NewCluster(cell, hostname)
+		defer clusterInstance.Teardown()
+
+		// Start topo server
+		if err := clusterInstance.StartTopo(); err != nil {
+			return 1
+		}
+
+		// Start keyspace
+		keyspace := &cluster.Keyspace{
+			Name:      keyspaceName,
+			SchemaSQL: sqlSchema,
+			VSchema:   vSchema,
+		}
+		if err := clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, false); err != nil {
+			return 1
+		}
+
+		// Start vtgate
+		if err := clusterInstance.StartVtgate(); err != nil {
+			return 1
+		}
+
+		return m.Run()
+	}()
+	os.Exit(exitCode)
+}
+
+func exec(t *testing.T, conn *mysql.Conn, query string) (*sqltypes.Result, error) {
+	t.Helper()
+	return conn.ExecuteFetch(query, 1000, true)
+}
diff --git a/go/test/endtoend/vtgate/setstatement/sysvar_test.go b/go/test/endtoend/vtgate/setstatement/sysvar_test.go
new file mode 100644
index 0000000000..3db7aaadb8
--- /dev/null
+++ b/go/test/endtoend/vtgate/setstatement/sysvar_test.go
@@ -0,0 +1,89 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package setstatement
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/test/endtoend/cluster"
+)
+
+func TestSetSysVar(t *testing.T) {
+	defer cluster.PanicHandler(t)
+	ctx := context.Background()
+	vtParams := mysql.ConnParams{
+		Host: "localhost",
+		Port: clusterInstance.VtgateMySQLPort,
+	}
+	type queriesWithExpectations struct {
+		query           string
+		expectedRows    string
+		rowsAffected    int
+		errMsg          string
+		expectedWarning 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\")]]",
+	}, {
+		query:        `set @@sql_mode = @@sql_mode`,
+		expectedRows: ``, rowsAffected: 0,
+	}, {
+		query:        `set @@sql_mode = concat(@@sql_mode,"")`,
+		expectedRows: ``, rowsAffected: 0,
+	}, {
+		query:  `set @@sql_mode = concat(@@sql_mode,"ALLOW_INVALID_DATES")`,
+		errMsg: "Modification not allowed using set construct for: sql_mode",
+	}}
+
+	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, err := exec(t, conn, "show warnings")
+					require.NoError(t, err)
+					if got, want := fmt.Sprintf("%v", qr.Rows), q.expectedWarning; got != want {
+						t.Errorf("select:\n%v want\n%v", got, want)
+					}
+				}
+			}
+		})
+	}
+}
diff --git a/go/test/endtoend/vtgate/setstatement/udv_test.go b/go/test/endtoend/vtgate/setstatement/udv_test.go
index 86baee4adc..8e2cff6e72 100644
--- a/go/test/endtoend/vtgate/setstatement/udv_test.go
+++ b/go/test/endtoend/vtgate/setstatement/udv_test.go
@@ -1,99 +1,34 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package setstatement
 
 import (
 	"context"
-	"flag"
 	"fmt"
-	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/stretchr/testify/assert"
-
 	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/test/endtoend/cluster"
 )
 
-var (
-	clusterInstance *cluster.LocalProcessCluster
-	keyspaceName    = "ks"
-	cell            = "zone1"
-	hostname        = "localhost"
-	sqlSchema       = `
-	create table test(
-		id bigint,
-		val1 varchar(16),
-		val2 int,
-		val3 float,
-		primary key(id)
-	)Engine=InnoDB;`
-
-	vSchema = `
-		{	
-			"sharded":true,
-			"vindexes": {
-				"hash_index": {
-					"type": "hash"
-				}
-			},	
-			"tables": {
-				"test":{
-					"column_vindexes": [
-						{
-							"column": "id",
-							"name": "hash_index"
-						}
-					]
-				}
-			}
-		}
-	`
-)
-
-func TestMain(m *testing.M) {
-	defer cluster.PanicHandler(nil)
-	flag.Parse()
-
-	exitCode := func() int {
-		clusterInstance = cluster.NewCluster(cell, hostname)
-		defer clusterInstance.Teardown()
-
-		// Start topo server
-		if err := clusterInstance.StartTopo(); err != nil {
-			return 1
-		}
-
-		// Start keyspace
-		keyspace := &cluster.Keyspace{
-			Name:      keyspaceName,
-			SchemaSQL: sqlSchema,
-			VSchema:   vSchema,
-		}
-		if err := clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, false); err != nil {
-			return 1
-		}
-
-		// Start vtgate
-		if err := clusterInstance.StartVtgate(); err != nil {
-			return 1
-		}
-
-		return m.Run()
-	}()
-	os.Exit(exitCode)
-}
-
-func exec(t *testing.T, conn *mysql.Conn, query string) *sqltypes.Result {
-	t.Helper()
-	qr, err := conn.ExecuteFetch(query, 1000, true)
-	require.Nil(t, err)
-	return qr
-}
-
-func TestSet(t *testing.T) {
+func TestSetUDV(t *testing.T) {
 	defer cluster.PanicHandler(t)
 	ctx := context.Background()
 	vtParams := mysql.ConnParams{
@@ -150,8 +85,9 @@ func TestSet(t *testing.T) {
 
 	for i, q := range queries {
 		t.Run(fmt.Sprintf("%d-%s", i, q.query), func(t *testing.T) {
-			qr := exec(t, conn, q.query)
-			assert.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query)
+			qr, err := exec(t, conn, q.query)
+			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 != "" {
diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go
index 2b7acc6d80..e9837e9f50 100644
--- a/go/vt/vtgate/engine/fake_vcursor_test.go
+++ b/go/vt/vtgate/engine/fake_vcursor_test.go
@@ -54,7 +54,9 @@ func (t noopVCursor) SetUDV(key string, value interface{}) error {
 func (t noopVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error {
 	panic("implement me")
 }
-
+func (t noopVCursor) Session() SessionActions {
+	return t
+}
 func (t noopVCursor) SetTarget(target string) error {
 	panic("implement me")
 }
@@ -136,6 +138,10 @@ func (f *loggingVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.D
 	panic("implement me")
 }
 
+func (f *loggingVCursor) Session() SessionActions {
+	return f
+}
+
 func (f *loggingVCursor) SetTarget(target string) error {
 	f.log = append(f.log, fmt.Sprintf("Target set to %s", target))
 	return nil
diff --git a/go/vt/vtgate/engine/send_test.go b/go/vt/vtgate/engine/send_test.go
index 5c8bf8efa0..4604ce0e48 100644
--- a/go/vt/vtgate/engine/send_test.go
+++ b/go/vt/vtgate/engine/send_test.go
@@ -1,3 +1,19 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package engine
 
 import (
diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go
new file mode 100644
index 0000000000..a243445b4c
--- /dev/null
+++ b/go/vt/vtgate/engine/set_test.go
@@ -0,0 +1,195 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package engine
+
+import (
+	"testing"
+
+	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/key"
+	"vitess.io/vitess/go/vt/vtgate/vindexes"
+
+	"github.com/stretchr/testify/require"
+
+	querypb "vitess.io/vitess/go/vt/proto/query"
+)
+
+func TestSetTable(t *testing.T) {
+	type testCase struct {
+		testName         string
+		setOps           []SetOp
+		qr               []*sqltypes.Result
+		expectedQueryLog []string
+		expectedWarning  []*querypb.QueryWarning
+		expectedError    string
+	}
+
+	tests := []testCase{
+		{
+			testName:         "nil set ops",
+			expectedQueryLog: []string{},
+		},
+		{
+			testName: "udv",
+			setOps: []SetOp{
+				&UserDefinedVariable{
+					Name: "x",
+					PlanValue: sqltypes.PlanValue{
+						Value: sqltypes.NewInt64(42),
+					},
+				},
+			},
+			expectedQueryLog: []string{
+				`UDV set with (x,INT64(42))`,
+			},
+		},
+		{
+			testName: "sysvar ignore",
+			setOps: []SetOp{
+				&SysVarIgnore{
+					Name: "x",
+					Expr: "42",
+				},
+			},
+			expectedWarning: []*querypb.QueryWarning{
+				{Code: 1235, Message: "Ignored inapplicable SET x = 42"},
+			},
+		},
+		{
+			testName: "sysvar check and ignore",
+			setOps: []SetOp{
+				&SysVarCheckAndIgnore{
+					Name: "x",
+					Keyspace: &vindexes.Keyspace{
+						Name:    "ks",
+						Sharded: true,
+					},
+					TargetDestination: key.DestinationAnyShard{},
+					CheckSysVarQuery:  "dummy_query",
+				},
+			},
+			qr: []*sqltypes.Result{sqltypes.MakeTestResult(
+				sqltypes.MakeTestFields(
+					"id",
+					"int64",
+				),
+				"1",
+			)},
+			expectedQueryLog: []string{
+				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
+				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
+			},
+		},
+		{
+			testName: "sysvar check and error",
+			setOps: []SetOp{
+				&SysVarCheckAndIgnore{
+					Name: "x",
+					Keyspace: &vindexes.Keyspace{
+						Name:    "ks",
+						Sharded: true,
+					},
+					TargetDestination: key.DestinationAnyShard{},
+					CheckSysVarQuery:  "dummy_query",
+				},
+			},
+			expectedQueryLog: []string{
+				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
+				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
+			},
+			expectedError: "Modification not allowed using set construct for: x",
+		},
+		{
+			testName: "sysvar checkAndIgnore multi destination error",
+			setOps: []SetOp{
+				&SysVarCheckAndIgnore{
+					Name: "x",
+					Keyspace: &vindexes.Keyspace{
+						Name:    "ks",
+						Sharded: true,
+					},
+					TargetDestination: key.DestinationAllShards{},
+					CheckSysVarQuery:  "dummy_query",
+				},
+			},
+			expectedQueryLog: []string{
+				`ResolveDestinations ks [] Destinations:DestinationAllShards()`,
+			},
+			expectedError: "Unexpected error, DestinationKeyspaceID mapping to multiple shards: DestinationAllShards()",
+		},
+		{
+			testName: "udv_ignr_chignr",
+			setOps: []SetOp{
+				&UserDefinedVariable{
+					Name: "x",
+					PlanValue: sqltypes.PlanValue{
+						Value: sqltypes.NewInt64(1),
+					},
+				},
+				&SysVarIgnore{
+					Name: "y",
+					Expr: "2",
+				},
+				&SysVarCheckAndIgnore{
+					Name: "z",
+					Keyspace: &vindexes.Keyspace{
+						Name:    "ks",
+						Sharded: true,
+					},
+					TargetDestination: key.DestinationAnyShard{},
+					CheckSysVarQuery:  "dummy_query",
+				},
+			},
+			expectedQueryLog: []string{
+				`UDV set with (x,INT64(1))`,
+				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
+				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
+			},
+			expectedWarning: []*querypb.QueryWarning{
+				{Code: 1235, Message: "Ignored inapplicable SET y = 2"},
+			},
+			qr: []*sqltypes.Result{sqltypes.MakeTestResult(
+				sqltypes.MakeTestFields(
+					"id",
+					"int64",
+				),
+				"1",
+			)},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.testName, func(t *testing.T) {
+			set := &Set{
+				Ops: tc.setOps,
+			}
+			vc := &loggingVCursor{
+				shards:  []string{"-20", "20-"},
+				results: tc.qr,
+			}
+			_, err := set.Execute(vc, map[string]*querypb.BindVariable{}, false)
+			if tc.expectedError == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tc.expectedError)
+			}
+
+			vc.ExpectLog(t, tc.expectedQueryLog)
+			vc.ExpectWarnings(t, tc.expectedWarning)
+		})
+	}
+}
diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go
index 5dc1423de3..6cf2316896 100644
--- a/go/vt/vtgate/executor_set_test.go
+++ b/go/vt/vtgate/executor_set_test.go
@@ -117,7 +117,7 @@ func TestExecutorSet(t *testing.T) {
 		err: "unsupported in set: global",
 	}, {
 		in:  "set global @@session.client_found_rows = 1",
-		err: "unsupported in set: mixed using of variable scope",
+		err: "unsupported in set: global",
 	}, {
 		in:  "set client_found_rows = 'aa'",
 		err: "unexpected value type for client_found_rows: string",
diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
index 509e14b37f..27f7105409 100644
--- a/go/vt/vtgate/planbuilder/plan_test.go
+++ b/go/vt/vtgate/planbuilder/plan_test.go
@@ -170,6 +170,7 @@ func TestPlan(t *testing.T) {
 	testFile(t, "memory_sort_cases.txt", testOutputTempDir, vschemaWrapper)
 	testFile(t, "use_cases.txt", testOutputTempDir, vschemaWrapper)
 	testFile(t, "set_cases.txt", testOutputTempDir, vschemaWrapper)
+	testFile(t, "set_sysvar_cases.txt", testOutputTempDir, vschemaWrapper)
 }
 
 func TestOne(t *testing.T) {
diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
index 99d4f72e92..b790194c7c 100644
--- a/go/vt/vtgate/planbuilder/testdata/set_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
@@ -8,6 +8,7 @@
     "Variant": "",
     "Ops": [
       {
+        "Type": "UserDefinedVariable",
         "Name": "foo",
         "PlanValue": 42
       }
@@ -25,10 +26,12 @@
     "Variant": "",
     "Ops": [
       {
+        "Type": "UserDefinedVariable",
         "Name": "foo",
         "PlanValue": 42
       },
       {
+        "Type": "UserDefinedVariable",
         "Name": "bar",
         "PlanValue": ":__vtudvfoo"
       }
diff --git a/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
new file mode 100644
index 0000000000..5d554ab27a
--- /dev/null
+++ b/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
@@ -0,0 +1,44 @@
+# set ignore plan
+"set @@default_storage_engine = 'DONOTCHANGEME'"
+{
+  "QueryType": "SET",
+  "Original": "set @@default_storage_engine = 'DONOTCHANGEME'",
+  "Instructions": {
+    "OperatorType": "Set",
+    "Variant": "",
+    "Ops": [
+      {
+        "Type": "SysVarIgnore",
+        "Name": "default_storage_engine",
+        "Expr": "'DONOTCHANGEME'"
+      }
+    ]
+  }
+}
+
+# set check and ignore plan
+"set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')"
+{
+  "QueryType": "SET",
+  "Original": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')",
+  "Instructions": {
+    "OperatorType": "Set",
+    "Variant": "",
+    "Ops": [
+      {
+        "Type": "SysVarCheckAndIgnore",
+        "Name": "sql_mode",
+        "Keyspace": {
+          "Name": "main",
+          "Sharded": false
+        },
+        "TargetDestination": {},
+        "CheckSysVarQuery": "select 1 from dual where @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')"
+      }
+    ]
+  }
+}
+
+# set plan building not supported
+"set @@innodb_strict_mode = OFF"
+"plan building not supported"
diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
index b9173fb409..d2bc6dac7d 100644
--- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
@@ -11,7 +11,7 @@
 "expression is too complex ':__vtudvfoo + 1'"
 
 # set user defined and system variable
-"set @foo = 42, @bar = @foo, @@sql_mode = @@sql_mode"
+"set @foo = 42, @bar = @foo, @@wait_timeout = 28800"
 "plan building not supported"
 
 # SHOW