Repository: vitessio/vitess
Pull request #6586 (2020-08-18 09:37:53)

ForwardPort: Passthrough System Variables enabled on flag
enabled as default on master
disabled as default on release 7.0 branch

#6562 


Patch:
diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go
index 6b73b22be8..e9d547613c 100644
--- a/go/vt/vtgate/planbuilder/builder.go
+++ b/go/vt/vtgate/planbuilder/builder.go
@@ -132,6 +132,7 @@ type ContextVSchema interface {
 	TargetDestination(qualifier string) (key.Destination, *vindexes.Keyspace, topodatapb.TabletType, error)
 	AnyKeyspace() (*vindexes.Keyspace, error)
 	FirstSortedKeyspace() (*vindexes.Keyspace, error)
+	SysVarSetEnabled() bool
 }
 
 //-------------------------------------------------------------------------
diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
index d66aa3bc99..952abc77c8 100644
--- a/go/vt/vtgate/planbuilder/set.go
+++ b/go/vt/vtgate/planbuilder/set.go
@@ -330,18 +330,22 @@ func buildSetOpIgnore(boolean bool) func(*sqlparser.SetExpr, ContextVSchema) (en
 
 func buildSetOpCheckAndIgnore(boolean bool) func(*sqlparser.SetExpr, ContextVSchema) (engine.SetOp, error) {
 	return func(expr *sqlparser.SetExpr, schema ContextVSchema) (engine.SetOp, error) {
-		keyspace, dest, err := resolveDestination(schema)
-		if err != nil {
-			return nil, err
-		}
+		return planSysVarCheckIgnore(expr, schema, boolean)
+	}
+}
 
-		return &engine.SysVarCheckAndIgnore{
-			Name:              expr.Name.Lowered(),
-			Keyspace:          keyspace,
-			TargetDestination: dest,
-			Expr:              extractValue(expr, boolean),
-		}, nil
+func planSysVarCheckIgnore(expr *sqlparser.SetExpr, schema ContextVSchema, boolean bool) (engine.SetOp, error) {
+	keyspace, dest, err := resolveDestination(schema)
+	if err != nil {
+		return nil, err
 	}
+
+	return &engine.SysVarCheckAndIgnore{
+		Name:              expr.Name.Lowered(),
+		Keyspace:          keyspace,
+		TargetDestination: dest,
+		Expr:              extractValue(expr, boolean),
+	}, nil
 }
 
 func expressionOkToDelegateToTablet(e sqlparser.Expr) bool {
@@ -362,7 +366,12 @@ func expressionOkToDelegateToTablet(e sqlparser.Expr) bool {
 }
 
 func buildSetOpVarSet(boolean bool) func(*sqlparser.SetExpr, ContextVSchema) (engine.SetOp, error) {
+
 	return func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
+		if !vschema.SysVarSetEnabled() {
+			return planSysVarCheckIgnore(expr, vschema, boolean)
+		}
+
 		ks, err := vschema.AnyKeyspace()
 		if err != nil {
 			return nil, err
diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go
index 940385653d..3a501e8de2 100644
--- a/go/vt/vtgate/vcursor_impl.go
+++ b/go/vt/vtgate/vcursor_impl.go
@@ -473,6 +473,10 @@ func (vc *vcursorImpl) TargetDestination(qualifier string) (key.Destination, *vi
 	return vc.destination, keyspace.Keyspace, vc.tabletType, nil
 }
 
+func (vc *vcursorImpl) SysVarSetEnabled() bool {
+	return *sysVarSetEnabled
+}
+
 // ParseDestinationTarget parses destination target string and sets default keyspace if possible.
 func parseDestinationTarget(targetString string, vschema *vindexes.VSchema) (string, topodatapb.TabletType, key.Destination, error) {
 	destKeyspace, destTabletType, dest, err := topoprotopb.ParseDestination(targetString, defaultTabletType)
diff --git a/go/vt/vtgate/vtgate.go b/go/vt/vtgate/vtgate.go
index 33c0e0a1e6..bb9a6ebb8f 100644
--- a/go/vt/vtgate/vtgate.go
+++ b/go/vt/vtgate/vtgate.go
@@ -69,6 +69,9 @@ var (
 	HealthCheckTimeout = flag.Duration("healthcheck_timeout", time.Minute, "the health check timeout period")
 	maxPayloadSize     = flag.Int("max_payload_size", 0, "The threshold for query payloads in bytes. A payload greater than this threshold will result in a failure to handle the query.")
 	warnPayloadSize    = flag.Int("warn_payload_size", 0, "The warning threshold for query payloads in bytes. A payload greater than this threshold will cause the VtGateWarnings.WarnPayloadSizeExceeded counter to be incremented.")
+
+	// Put set-passthrough under a flag.
+	sysVarSetEnabled = flag.Bool("enable_system_settings", true, "This will enable the system settings to be changed per session at the database connection level")
 )
 
 func getTxMode() vtgatepb.TransactionMode {


Test patch:
diff --git a/go/test/endtoend/cluster/vtgate_process.go b/go/test/endtoend/cluster/vtgate_process.go
index f01d38fb89..ed2c287304 100644
--- a/go/test/endtoend/cluster/vtgate_process.go
+++ b/go/test/endtoend/cluster/vtgate_process.go
@@ -53,6 +53,7 @@ type VtgateProcess struct {
 	MySQLAuthServerImpl   string
 	Directory             string
 	VerifyURL             string
+	SysVarSetEnabled      bool
 	//Extra Args to be set before starting the vtgate process
 	ExtraArgs []string
 
@@ -63,8 +64,7 @@ type VtgateProcess struct {
 // Setup starts Vtgate process with required arguements
 func (vtgate *VtgateProcess) Setup() (err error) {
 
-	vtgate.proc = exec.Command(
-		vtgate.Binary,
+	args := []string{
 		"-topo_implementation", vtgate.CommonArg.TopoImplementation,
 		"-topo_global_server_address", vtgate.CommonArg.TopoGlobalAddress,
 		"-topo_global_root", vtgate.CommonArg.TopoGlobalRoot,
@@ -80,6 +80,13 @@ func (vtgate *VtgateProcess) Setup() (err error) {
 		"-gateway_implementation", vtgate.GatewayImplementation,
 		"-service_map", vtgate.ServiceMap,
 		"-mysql_auth_server_impl", vtgate.MySQLAuthServerImpl,
+	}
+	if vtgate.SysVarSetEnabled {
+		args = append(args, "-enable_system_settings")
+	}
+	vtgate.proc = exec.Command(
+		vtgate.Binary,
+		args...,
 	)
 	if *isCoverage {
 		vtgate.proc.Args = append(vtgate.proc.Args, "-test.coverprofile="+getCoveragePath("vtgate.out"))
diff --git a/go/test/endtoend/vtgate/setstatement/main_test.go b/go/test/endtoend/vtgate/setstatement/main_test.go
index 8cf8d798ab..0cfe9c0281 100644
--- a/go/test/endtoend/vtgate/setstatement/main_test.go
+++ b/go/test/endtoend/vtgate/setstatement/main_test.go
@@ -120,7 +120,9 @@ func TestMain(m *testing.M) {
 		}
 
 		// Start vtgate
-		if err := clusterInstance.StartVtgate(); err != nil {
+		vtgateProcess := clusterInstance.NewVtgateInstance()
+		vtgateProcess.SysVarSetEnabled = true
+		if err := vtgateProcess.Setup(); err != nil {
 			return 1
 		}
 
diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go
index b8d61d5d35..86e6181795 100644
--- a/go/vt/vtgate/executor_set_test.go
+++ b/go/vt/vtgate/executor_set_test.go
@@ -250,6 +250,7 @@ func TestExecutorSet(t *testing.T) {
 
 func TestExecutorSetOp(t *testing.T) {
 	executor, _, _, sbclookup := createLegacyExecutorEnv()
+	*sysVarSetEnabled = true
 
 	sbclookup.SetResults([]*sqltypes.Result{
 		sqltypes.MakeTestResult(sqltypes.MakeTestFields("sql_mode", "varchar"), "STRICT_ALL_TABLES,NO_AUTO_UPDATES"),
diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
index b307f4c4d3..4afb969fca 100644
--- a/go/vt/vtgate/planbuilder/plan_test.go
+++ b/go/vt/vtgate/planbuilder/plan_test.go
@@ -147,7 +147,8 @@ func init() {
 
 func TestPlan(t *testing.T) {
 	vschemaWrapper := &vschemaWrapper{
-		v: loadSchema(t, "schema_test.json"),
+		v:             loadSchema(t, "schema_test.json"),
+		sysVarEnabled: true,
 	}
 
 	testOutputTempDir, err := ioutil.TempDir("", "plan_test")
@@ -178,6 +179,18 @@ func TestPlan(t *testing.T) {
 	testFile(t, "lock_cases.txt", testOutputTempDir, vschemaWrapper)
 }
 
+func TestSysVarSetDisabled(t *testing.T) {
+	vschemaWrapper := &vschemaWrapper{
+		v:             loadSchema(t, "schema_test.json"),
+		sysVarEnabled: false,
+	}
+
+	testOutputTempDir, err := ioutil.TempDir("", "plan_test")
+	require.NoError(t, err)
+	defer os.RemoveAll(testOutputTempDir)
+	testFile(t, "set_sysvar_disabled_cases.txt", testOutputTempDir, vschemaWrapper)
+}
+
 func TestOne(t *testing.T) {
 	vschema := &vschemaWrapper{
 		v: loadSchema(t, "schema_test.json"),
@@ -258,10 +271,15 @@ func loadSchema(t *testing.T, filename string) *vindexes.VSchema {
 var _ ContextVSchema = (*vschemaWrapper)(nil)
 
 type vschemaWrapper struct {
-	v          *vindexes.VSchema
-	keyspace   *vindexes.Keyspace
-	tabletType topodatapb.TabletType
-	dest       key.Destination
+	v             *vindexes.VSchema
+	keyspace      *vindexes.Keyspace
+	tabletType    topodatapb.TabletType
+	dest          key.Destination
+	sysVarEnabled bool
+}
+
+func (vw *vschemaWrapper) SysVarSetEnabled() bool {
+	return vw.sysVarEnabled
 }
 
 func (vw *vschemaWrapper) TargetDestination(qualifier string) (key.Destination, *vindexes.Keyspace, topodatapb.TabletType, error) {
diff --git a/go/vt/vtgate/planbuilder/testdata/set_sysvar_disabled_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_sysvar_disabled_cases.txt
new file mode 100644
index 0000000000..f213ba1a96
--- /dev/null
+++ b/go/vt/vtgate/planbuilder/testdata/set_sysvar_disabled_cases.txt
@@ -0,0 +1,36 @@
+# set passthrough disabled - check and ignore plan
+"set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER'), @@sql_safe_updates = 1"
+{
+  "QueryType": "SET",
+  "Original": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER'), @@sql_safe_updates = 1",
+  "Instructions": {
+    "OperatorType": "Set",
+    "Ops": [
+      {
+        "Type": "SysVarCheckAndIgnore",
+        "Name": "sql_mode",
+        "Keyspace": {
+          "Name": "main",
+          "Sharded": false
+        },
+        "TargetDestination": {},
+        "Expr": "concat(@@sql_mode, ',NO_AUTO_CREATE_USER')"
+      },
+      {
+        "Type": "SysVarCheckAndIgnore",
+        "Name": "sql_safe_updates",
+        "Keyspace": {
+          "Name": "main",
+          "Sharded": false
+        },
+        "TargetDestination": {},
+        "Expr": "1"
+      }
+    ],
+    "Inputs": [
+      {
+        "OperatorType": "SingleRow"
+      }
+    ]
+  }
+}