Repository: vitessio/vitess
Pull request #6501 (2020-07-30 09:50:45)

handle boolean settings better


Patch:
diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go
index 1233faeb7b..780414b0c4 100644
--- a/go/vt/vtgate/executor.go
+++ b/go/vt/vtgate/executor.go
@@ -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"
@@ -504,18 +503,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 {
@@ -526,17 +513,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 {
@@ -579,34 +555,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 {
diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
index fe8f75cb66..d66aa3bc99 100644
--- a/go/vt/vtgate/planbuilder/set.go
+++ b/go/vt/vtgate/planbuilder/set.go
@@ -31,164 +31,175 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/evalengine"
 )
 
-type planFunc = func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error)
+type (
+	planFunc = func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error)
+
+	expressionConverter struct {
+		tabletExpressions []*sqlparser.SetExpr
+	}
+
+	setting struct {
+		name    string
+		boolean bool
+	}
+)
 
 var sysVarPlanningFunc = map[string]planFunc{}
 
-var notSupported = []string{
-	"audit_log_read_buffer_size",
-	"auto_increment_increment",
-	"auto_increment_offset",
-	"binlog_direct_non_transactional_updates",
-	"binlog_row_image",
-	"binlog_rows_query_log_events",
-	"innodb_ft_enable_stopword",
-	"innodb_ft_user_stopword_table",
-	"max_points_in_geometry",
-	"max_sp_recursion_depth",
-	"myisam_repair_threads",
-	"myisam_sort_buffer_size",
-	"myisam_stats_method",
-	"ndb_allow_copying_alter_table",
-	"ndb_autoincrement_prefetch_sz",
-	"ndb_blob_read_batch_bytes",
-	"ndb_blob_write_batch_bytes",
-	"ndb_deferred_constraints",
-	"ndb_force_send",
-	"ndb_fully_replicated",
-	"ndb_index_stat_enable",
-	"ndb_index_stat_option",
-	"ndb_join_pushdown",
-	"ndb_log_bin",
-	"ndb_log_exclusive_reads",
-	"ndb_row_checksum",
-	"ndb_use_exact_count",
-	"ndb_use_transactions",
-	"ndbinfo_max_bytes",
-	"ndbinfo_max_rows",
-	"ndbinfo_show_hidden",
-	"ndbinfo_table_prefix",
-	"old_alter_table",
-	"preload_buffer_size",
-	"rbr_exec_mode",
-	"sql_log_off",
-	"thread_pool_high_priority_connection",
-	"thread_pool_prio_kickup_timer",
-	"transaction_write_set_extraction",
+var notSupported = []setting{
+	{name: "audit_log_read_buffer_size"},
+	{name: "auto_increment_increment"},
+	{name: "auto_increment_offset"},
+	{name: "binlog_direct_non_transactional_updates"},
+	{name: "binlog_row_image"},
+	{name: "binlog_rows_query_log_events"},
+	{name: "innodb_ft_enable_stopword"},
+	{name: "innodb_ft_user_stopword_table"},
+	{name: "max_points_in_geometry"},
+	{name: "max_sp_recursion_depth"},
+	{name: "myisam_repair_threads"},
+	{name: "myisam_sort_buffer_size"},
+	{name: "myisam_stats_method"},
+	{name: "ndb_allow_copying_alter_table"},
+	{name: "ndb_autoincrement_prefetch_sz"},
+	{name: "ndb_blob_read_batch_bytes"},
+	{name: "ndb_blob_write_batch_bytes"},
+	{name: "ndb_deferred_constraints"},
+	{name: "ndb_force_send"},
+	{name: "ndb_fully_replicated"},
+	{name: "ndb_index_stat_enable"},
+	{name: "ndb_index_stat_option"},
+	{name: "ndb_join_pushdown"},
+	{name: "ndb_log_bin"},
+	{name: "ndb_log_exclusive_reads"},
+	{name: "ndb_row_checksum"},
+	{name: "ndb_use_exact_count"},
+	{name: "ndb_use_transactions"},
+	{name: "ndbinfo_max_bytes"},
+	{name: "ndbinfo_max_rows"},
+	{name: "ndbinfo_show_hidden"},
+	{name: "ndbinfo_table_prefix"},
+	{name: "old_alter_table"},
+	{name: "preload_buffer_size"},
+	{name: "rbr_exec_mode"},
+	{name: "sql_log_off"},
+	{name: "thread_pool_high_priority_connection"},
+	{name: "thread_pool_prio_kickup_timer"},
+	{name: "transaction_write_set_extraction"},
 }
 
-var ignoreThese = []string{
-	"big_tables",
-	"bulk_insert_buffer_size",
-	"debug",
-	"default_storage_engine",
-	"default_tmp_storage_engine",
-	"innodb_strict_mode",
-	"innodb_support_xa",
-	"innodb_table_locks",
-	"innodb_tmpdir",
-	"join_buffer_size",
-	"keep_files_on_create",
-	"lc_messages",
-	"long_query_time",
-	"low_priority_updates",
-	"max_delayed_threads",
-	"max_insert_delayed_threads",
-	"multi_range_count",
-	"net_buffer_length",
-	"new",
-	"query_cache_type",
-	"query_cache_wlock_invalidate",
-	"query_prealloc_size",
-	"sql_buffer_result",
-	"transaction_alloc_block_size",
-	"wait_timeout",
+var ignoreThese = []setting{
+	{name: "big_tables", boolean: true},
+	{name: "bulk_insert_buffer_size"},
+	{name: "debug"},
+	{name: "default_storage_engine"},
+	{name: "default_tmp_storage_engine"},
+	{name: "innodb_strict_mode", boolean: true},
+	{name: "innodb_support_xa", boolean: true},
+	{name: "innodb_table_locks", boolean: true},
+	{name: "innodb_tmpdir"},
+	{name: "join_buffer_size"},
+	{name: "keep_files_on_create", boolean: true},
+	{name: "lc_messages"},
+	{name: "long_query_time"},
+	{name: "low_priority_updates", boolean: true},
+	{name: "max_delayed_threads"},
+	{name: "max_insert_delayed_threads"},
+	{name: "multi_range_count"},
+	{name: "net_buffer_length"},
+	{name: "new", boolean: true},
+	{name: "query_cache_type"},
+	{name: "query_cache_wlock_invalidate", boolean: true},
+	{name: "query_prealloc_size"},
+	{name: "sql_buffer_result", boolean: true},
+	{name: "transaction_alloc_block_size"},
+	{name: "wait_timeout"},
 }
 
-var useReservedConn = []string{
-	"default_week_format",
-	"end_markers_in_json",
-	"eq_range_index_dive_limit",
-	"explicit_defaults_for_timestamp",
-	"foreign_key_checks",
-	"group_concat_max_len",
-	"max_heap_table_size",
-	"max_seeks_for_key",
-	"max_tmp_tables",
-	"min_examined_row_limit",
-	"old_passwords",
-	"optimizer_prune_level",
-	"optimizer_search_depth",
-	"optimizer_switch",
-	"optimizer_trace",
-	"optimizer_trace_features",
-	"optimizer_trace_limit",
-	"optimizer_trace_max_mem_size",
-	"transaction_isolation",
-	"tx_isolation",
-	"optimizer_trace_offset",
-	"parser_max_mem_size",
-	"profiling",
-	"profiling_history_size",
-	"query_alloc_block_size",
-	"range_alloc_block_size",
-	"range_optimizer_max_mem_size",
-	"read_buffer_size",
-	"read_rnd_buffer_size",
-	"show_create_table_verbosity",
-	"show_old_temporals",
-	"sort_buffer_size",
-	"sql_big_selects",
-	"sql_mode",
-	"sql_notes",
-	"sql_quote_show_create",
-	"sql_safe_updates",
-	"sql_warnings",
-	"tmp_table_size",
-	"transaction_prealloc_size",
-	"unique_checks",
-	"updatable_views_with_limit",
+var useReservedConn = []setting{
+	{name: "default_week_format"},
+	{name: "end_markers_in_json", boolean: true},
+	{name: "eq_range_index_dive_limit"},
+	{name: "explicit_defaults_for_timestamp"},
+	{name: "foreign_key_checks", boolean: true},
+	{name: "group_concat_max_len"},
+	{name: "max_heap_table_size"},
+	{name: "max_seeks_for_key"},
+	{name: "max_tmp_tables"},
+	{name: "min_examined_row_limit"},
+	{name: "old_passwords"},
+	{name: "optimizer_prune_level"},
+	{name: "optimizer_search_depth"},
+	{name: "optimizer_switch"},
+	{name: "optimizer_trace"},
+	{name: "optimizer_trace_features"},
+	{name: "optimizer_trace_limit"},
+	{name: "optimizer_trace_max_mem_size"},
+	{name: "transaction_isolation"},
+	{name: "tx_isolation"},
+	{name: "optimizer_trace_offset"},
+	{name: "parser_max_mem_size"},
+	{name: "profiling", boolean: true},
+	{name: "profiling_history_size"},
+	{name: "query_alloc_block_size"},
+	{name: "range_alloc_block_size"},
+	{name: "range_optimizer_max_mem_size"},
+	{name: "read_buffer_size"},
+	{name: "read_rnd_buffer_size"},
+	{name: "show_create_table_verbosity", boolean: true},
+	{name: "show_old_temporals", boolean: true},
+	{name: "sort_buffer_size"},
+	{name: "sql_big_selects", boolean: true},
+	{name: "sql_mode"},
+	{name: "sql_notes", boolean: true},
+	{name: "sql_quote_show_create", boolean: true},
+	{name: "sql_safe_updates", boolean: true},
+	{name: "sql_warnings", boolean: true},
+	{name: "tmp_table_size"},
+	{name: "transaction_prealloc_size"},
+	{name: "unique_checks", boolean: true},
+	{name: "updatable_views_with_limit", boolean: true},
 }
 
 // TODO: Most of these settings should be moved into SysSetOpAware, and change Vitess behaviour.
 // Until then, SET statements against these settings are allowed
 // as long as they have the same value as the underlying database
-var checkAndIgnore = []string{
-	"binlog_format",
-	"block_encryption_mode",
-	"character_set_client",
-	"character_set_connection",
-	"character_set_database",
-	"character_set_filesystem",
-	"character_set_results",
-	"character_set_server",
-	"collation_connection",
-	"collation_database",
-	"collation_server",
-	"completion_type",
-	"div_precision_increment",
-	"innodb_lock_wait_timeout",
-	"interactive_timeout",
-	"lc_time_names",
-	"lock_wait_timeout",
-	"max_allowed_packet",
-	"max_error_count",
-	"max_execution_time",
-	"max_join_size",
-	"max_length_for_sort_data",
-	"max_sort_length",
-	"max_user_connections",
-	"net_read_timeout",
-	"net_retry_count",
-	"net_write_timeout",
-	"session_track_gtids",
-	"session_track_schema",
-	"session_track_state_change",
-	"session_track_system_variables",
-	"session_track_transaction_info",
-	"sql_auto_is_null",
-	"time_zone",
-	"version_tokens_session",
+var checkAndIgnore = []setting{
+	{name: "binlog_format"},
+	{name: "block_encryption_mode"},
+	{name: "character_set_client"},
+	{name: "character_set_connection"},
+	{name: "character_set_database"},
+	{name: "character_set_filesystem"},
+	{name: "character_set_results"},
+	{name: "character_set_server"},
+	{name: "collation_connection"},
+	{name: "collation_database"},
+	{name: "collation_server"},
+	{name: "completion_type"},
+	{name: "div_precision_increment"},
+	{name: "innodb_lock_wait_timeout"},
+	{name: "interactive_timeout"},
+	{name: "lc_time_names"},
+	{name: "lock_wait_timeout"},
+	{name: "max_allowed_packet"},
+	{name: "max_error_count"},
+	{name: "max_execution_time"},
+	{name: "max_join_size"},
+	{name: "max_length_for_sort_data"},
+	{name: "max_sort_length"},
+	{name: "max_user_connections"},
+	{name: "net_read_timeout"},
+	{name: "net_retry_count"},
+	{name: "net_write_timeout"},
+	{name: "session_track_gtids"},
+	{name: "session_track_schema", boolean: true},
+	{name: "session_track_state_change", boolean: true},
+	{name: "session_track_system_variables"},
+	{name: "session_track_transaction_info"},
+	{name: "sql_auto_is_null", boolean: true},
+	{name: "time_zone"},
+	{name: "version_tokens_session"},
 }
 
 func init() {
@@ -198,12 +209,12 @@ func init() {
 	forSettings(notSupported, buildNotSupported)
 }
 
-func forSettings(settings []string, f planFunc) {
+func forSettings(settings []setting, f func(bool) planFunc) {
 	for _, setting := range settings {
-		if _, alreadyExists := sysVarPlanningFunc[setting]; alreadyExists {
-			panic("bug in set plan init - " + setting + " aleady configured")
+		if _, alreadyExists := sysVarPlanningFunc[setting.name]; alreadyExists {
+			panic("bug in set plan init - " + setting.name + " aleady configured")
 		}
-		sysVarPlanningFunc[setting] = f
+		sysVarPlanningFunc[setting.name] = f(setting.boolean)
 	}
 }
 
@@ -259,10 +270,6 @@ func buildSetPlan(stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive
 	}, nil
 }
 
-type expressionConverter struct {
-	tabletExpressions []*sqlparser.SetExpr
-}
-
 func (spb *expressionConverter) convert(setExpr *sqlparser.SetExpr) (evalengine.Expr, error) {
 	astExpr := setExpr.Expr
 	evalExpr, err := sqlparser.Convert(astExpr)
@@ -306,29 +313,35 @@ func (spb *expressionConverter) source(vschema ContextVSchema) (engine.Primitive
 	return primitive, nil
 }
 
-func buildNotSupported(e *sqlparser.SetExpr, _ ContextVSchema) (engine.SetOp, error) {
-	return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s: system setting is not supported", e.Name)
+func buildNotSupported(bool) func(*sqlparser.SetExpr, ContextVSchema) (engine.SetOp, error) {
+	return func(expr *sqlparser.SetExpr, schema ContextVSchema) (engine.SetOp, error) {
+		return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s: system setting is not supported", expr.Name)
+	}
 }
 
-func buildSetOpIgnore(expr *sqlparser.SetExpr, _ ContextVSchema) (engine.SetOp, error) {
-	return &engine.SysVarIgnore{
-		Name: expr.Name.Lowered(),
-		Expr: extractValue(expr),
-	}, nil
+func buildSetOpIgnore(boolean bool) func(*sqlparser.SetExpr, ContextVSchema) (engine.SetOp, error) {
+	return func(expr *sqlparser.SetExpr, _ ContextVSchema) (engine.SetOp, error) {
+		return &engine.SysVarIgnore{
+			Name: expr.Name.Lowered(),
+			Expr: extractValue(expr, boolean),
+		}, nil
+	}
 }
 
-func buildSetOpCheckAndIgnore(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
-	keyspace, dest, err := resolveDestination(vschema)
-	if err != nil {
-		return nil, err
-	}
+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 &engine.SysVarCheckAndIgnore{
-		Name:              expr.Name.Lowered(),
-		Keyspace:          keyspace,
-		TargetDestination: dest,
-		Expr:              extractValue(expr),
-	}, nil
+		return &engine.SysVarCheckAndIgnore{
+			Name:              expr.Name.Lowered(),
+			Keyspace:          keyspace,
+			TargetDestination: dest,
+			Expr:              extractValue(expr, boolean),
+		}, nil
+	}
 }
 
 func expressionOkToDelegateToTablet(e sqlparser.Expr) bool {
@@ -348,18 +361,20 @@ func expressionOkToDelegateToTablet(e sqlparser.Expr) bool {
 	return valid
 }
 
-func buildSetOpVarSet(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
-	ks, err := vschema.AnyKeyspace()
-	if err != nil {
-		return nil, err
-	}
+func buildSetOpVarSet(boolean bool) func(*sqlparser.SetExpr, ContextVSchema) (engine.SetOp, error) {
+	return func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
+		ks, err := vschema.AnyKeyspace()
+		if err != nil {
+			return nil, err
+		}
 
-	return &engine.SysVarSet{
-		Name:              expr.Name.Lowered(),
-		Keyspace:          ks,
-		TargetDestination: vschema.Destination(),
-		Expr:              extractValue(expr),
-	}, nil
+		return &engine.SysVarSet{
+			Name:              expr.Name.Lowered(),
+			Keyspace:          ks,
+			TargetDestination: vschema.Destination(),
+			Expr:              extractValue(expr, boolean),
+		}, nil
+	}
 }
 
 func resolveDestination(vschema ContextVSchema) (*vindexes.Keyspace, key.Destination, error) {
@@ -375,15 +390,32 @@ func resolveDestination(vschema ContextVSchema) (*vindexes.Keyspace, key.Destina
 	return keyspace, dest, nil
 }
 
-func extractValue(expr *sqlparser.SetExpr) string {
-	value := sqlparser.String(expr.Expr)
-	switch colname := expr.Expr.(type) {
+func extractValue(expr *sqlparser.SetExpr, boolean bool) string {
+	switch node := expr.Expr.(type) {
+	case *sqlparser.SQLVal:
+		if node.Type == sqlparser.StrVal && boolean {
+			switch strings.ToLower(string(node.Val)) {
+			case "on":
+				return "1"
+			case "off":
+				return "0"
+			}
+		}
 	case *sqlparser.ColName:
-		if colname.Name.AtCount() == sqlparser.NoAt {
-			value = fmt.Sprintf("'%s'", value)
+		// this is a little of a hack. it's used when the setting is not a normal expression, but rather
+		// an enumeration, such as utf8, utf8mb4, etc
+		if node.Name.AtCount() == sqlparser.NoAt {
+			switch node.Name.Lowered() {
+			case "on":
+				return "1"
+			case "off":
+				return "0"
+			}
+			return fmt.Sprintf("'%s'", sqlparser.String(expr.Expr))
 		}
 	}
-	return value
+
+	return sqlparser.String(expr.Expr)
 }
 
 // whitelist of functions knows to be safe to pass through to mysql for evaluation


Test patch:
diff --git a/go/test/endtoend/vtgate/setstatement/sysvar_test.go b/go/test/endtoend/vtgate/setstatement/sysvar_test.go
index 74d5342fc4..8c8d0713c4 100644
--- a/go/test/endtoend/vtgate/setstatement/sysvar_test.go
+++ b/go/test/endtoend/vtgate/setstatement/sysvar_test.go
@@ -59,11 +59,11 @@ func TestSetSysVarSingle(t *testing.T) {
 	}
 
 	queries := []queriesWithExpectations{{
-		name:     "default_storage_engine",
+		name:     "default_storage_engine", // ignored
 		expr:     "INNODB",
 		expected: `[[VARCHAR("InnoDB")]]`,
 	}, {
-		name:     "character_set_client",
+		name:     "character_set_client", // check and ignored
 		expr:     "utf8",
 		expected: `[[VARCHAR("utf8")]]`,
 	}, {
@@ -71,21 +71,29 @@ func TestSetSysVarSingle(t *testing.T) {
 		expr:     "@charvar",
 		expected: `[[VARCHAR("utf8")]]`,
 	}, {
-		name:     "sql_mode",
+		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)
diff --git a/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
index a2aa4ddcc7..c580cb86f5 100644
--- a/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
@@ -72,7 +72,7 @@
   }
 }
 
-# set plan building not supported
+# set plan building with ON/OFF enum
 "set @@innodb_strict_mode = OFF"
 {
   "QueryType": "SET",
@@ -83,7 +83,51 @@
       {
         "Type": "SysVarIgnore",
         "Name": "innodb_strict_mode",
-        "Expr": "'off'"
+        "Expr": "0"
+      }
+    ],
+    "Inputs": [
+      {
+        "OperatorType": "SingleRow"
+      }
+    ]
+  }
+}
+
+# set plan building with string literal
+"set @@innodb_strict_mode = 'OFF'"
+{
+  "QueryType": "SET",
+  "Original": "set @@innodb_strict_mode = 'OFF'",
+  "Instructions": {
+    "OperatorType": "Set",
+    "Ops": [
+      {
+        "Type": "SysVarIgnore",
+        "Name": "innodb_strict_mode",
+        "Expr": "0"
+      }
+    ],
+    "Inputs": [
+      {
+        "OperatorType": "SingleRow"
+      }
+    ]
+  }
+}
+
+# set plan building with string literal
+"set @@innodb_tmpdir = 'OFF'"
+{
+  "QueryType": "SET",
+  "Original": "set @@innodb_tmpdir = 'OFF'",
+  "Instructions": {
+    "OperatorType": "Set",
+    "Ops": [
+      {
+        "Type": "SysVarIgnore",
+        "Name": "innodb_tmpdir",
+        "Expr": "'OFF'"
       }
     ],
     "Inputs": [