Allow for dynamic configuration of DDL settings (vitessio#17328)

Signed-off-by: Dirkjan Bussink <[email protected]>

go/vt/vtgate/dynamicconfig/config.go

@@ -0,0 +1,6 @@
+package dynamicconfig
+
+type DDL interface {
+	OnlineEnabled() bool
+	DirectEnabled() bool
+}

go/vt/vtgate/engine/ddl.go

@@ -25,6 +25,7 @@ import (
 	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
@@ -42,8 +43,7 @@ type DDL struct {
 	NormalDDL *Send
 	OnlineDDL *OnlineDDL
 
-	DirectDDLEnabled bool
-	OnlineDDLEnabled bool
+	Config dynamicconfig.DDL
 
 	CreateTempTable bool
 }
@@ -107,12 +107,12 @@ func (ddl *DDL) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[st
 
 	switch {
 	case ddl.isOnlineSchemaDDL():
-		if !ddl.OnlineDDLEnabled {
+		if !ddl.Config.OnlineEnabled() {
 			return nil, schema.ErrOnlineDDLDisabled
 		}
 		return vcursor.ExecutePrimitive(ctx, ddl.OnlineDDL, bindVars, wantfields)
 	default: // non online-ddl
-		if !ddl.DirectDDLEnabled {
+		if !ddl.Config.DirectEnabled() {
 			return nil, schema.ErrDirectDDLDisabled
 		}
 		return vcursor.ExecutePrimitive(ctx, ddl.NormalDDL, bindVars, wantfields)

go/vt/vtgate/engine/ddl_test.go

@@ -27,13 +27,23 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
+type ddlConfig struct{}
+
+func (ddlConfig) DirectEnabled() bool {
+	return true
+}
+
+func (ddlConfig) OnlineEnabled() bool {
+	return true
+}
+
 func TestDDL(t *testing.T) {
 	ddl := &DDL{
 		DDL: &sqlparser.CreateTable{
 			Table: sqlparser.NewTableName("a"),
 		},
-		DirectDDLEnabled: true,
-		OnlineDDL:        &OnlineDDL{},
+		Config:    ddlConfig{},
+		OnlineDDL: &OnlineDDL{},
 		NormalDDL: &Send{
 			Keyspace: &vindexes.Keyspace{
 				Name:    "ks",

go/vt/vtgate/executor.go

@@ -1174,7 +1174,11 @@ func (e *Executor) buildStatement(
 	reservedVars *sqlparser.ReservedVars,
 	bindVarNeeds *sqlparser.BindVarNeeds,
 ) (*engine.Plan, error) {
-	plan, err := planbuilder.BuildFromStmt(ctx, query, stmt, reservedVars, vcursor, bindVarNeeds, enableOnlineDDL, enableDirectDDL)
+	cfg := &dynamicViperConfig{
+		onlineDDL: enableOnlineDDL,
+		directDDL: enableDirectDDL,
+	}
+	plan, err := planbuilder.BuildFromStmt(ctx, query, stmt, reservedVars, vcursor, bindVarNeeds, cfg)
 	if err != nil {
 		return nil, err
 	}

go/vt/vtgate/executor_ddl_test.go

@@ -27,8 +27,8 @@ import (
 
 func TestDDLFlags(t *testing.T) {
 	defer func() {
-		enableOnlineDDL = true
-		enableDirectDDL = true
+		enableOnlineDDL.Set(true)
+		enableDirectDDL.Set(true)
 	}()
 	testcases := []struct {
 		enableDirectDDL bool
@@ -57,8 +57,8 @@ func TestDDLFlags(t *testing.T) {
 		t.Run(fmt.Sprintf("%s-%v-%v", testcase.sql, testcase.enableDirectDDL, testcase.enableOnlineDDL), func(t *testing.T) {
 			executor, _, _, _, ctx := createExecutorEnv(t)
 			session := NewSafeSession(&vtgatepb.Session{TargetString: KsTestUnsharded})
-			enableDirectDDL = testcase.enableDirectDDL
-			enableOnlineDDL = testcase.enableOnlineDDL
+			enableDirectDDL.Set(testcase.enableDirectDDL)
+			enableOnlineDDL.Set(testcase.enableOnlineDDL)
 			_, err := executor.Execute(ctx, nil, "TestDDLFlags", session, testcase.sql, nil)
 			if testcase.wantErr {
 				require.EqualError(t, err, testcase.err)

go/vt/vtgate/planbuilder/builder.go

@@ -28,6 +28,7 @@ import (
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -63,6 +64,16 @@ func singleTable(ks, tbl string) string {
 	return fmt.Sprintf("%s.%s", ks, tbl)
 }
 
+type staticConfig struct{}
+
+func (staticConfig) OnlineEnabled() bool {
+	return true
+}
+
+func (staticConfig) DirectEnabled() bool {
+	return true
+}
+
 // TestBuilder builds a plan for a query based on the specified vschema.
 // This method is only used from tests
 func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error) {
@@ -92,12 +103,12 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e
 	}
 
 	reservedVars := sqlparser.NewReservedVars("vtg", reserved)
-	return BuildFromStmt(context.Background(), query, result.AST, reservedVars, vschema, result.BindVarNeeds, true, true)
+	return BuildFromStmt(context.Background(), query, result.AST, reservedVars, vschema, result.BindVarNeeds, staticConfig{})
 }
 
 // BuildFromStmt builds a plan based on the AST provided.
-func BuildFromStmt(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, bindVarNeeds *sqlparser.BindVarNeeds, enableOnlineDDL, enableDirectDDL bool) (*engine.Plan, error) {
-	planResult, err := createInstructionFor(ctx, query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+func BuildFromStmt(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, bindVarNeeds *sqlparser.BindVarNeeds, cfg dynamicconfig.DDL) (*engine.Plan, error) {
+	planResult, err := createInstructionFor(ctx, query, stmt, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -154,7 +165,7 @@ func buildRoutePlan(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVa
 	return f(stmt, reservedVars, vschema)
 }
 
-func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*planResult, error) {
+func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
 	switch stmt := stmt.(type) {
 	case *sqlparser.Select, *sqlparser.Insert, *sqlparser.Update, *sqlparser.Delete:
 		configuredPlanner, err := getConfiguredPlanner(vschema, stmt, query)
@@ -169,13 +180,13 @@ func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Stat
 		}
 		return buildRoutePlan(stmt, reservedVars, vschema, configuredPlanner)
 	case sqlparser.DDLStatement:
-		return buildGeneralDDLPlan(ctx, query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+		return buildGeneralDDLPlan(ctx, query, stmt, reservedVars, vschema, cfg)
 	case *sqlparser.AlterMigration:
-		return buildAlterMigrationPlan(query, stmt, vschema, enableOnlineDDL)
+		return buildAlterMigrationPlan(query, stmt, vschema, cfg)
 	case *sqlparser.RevertMigration:
-		return buildRevertMigrationPlan(query, stmt, vschema, enableOnlineDDL)
+		return buildRevertMigrationPlan(query, stmt, vschema, cfg)
 	case *sqlparser.ShowMigrationLogs:
-		return buildShowMigrationLogsPlan(query, vschema, enableOnlineDDL)
+		return buildShowMigrationLogsPlan(query, vschema, cfg)
 	case *sqlparser.ShowThrottledApps:
 		return buildShowThrottledAppsPlan(query, vschema)
 	case *sqlparser.ShowThrottlerStatus:
@@ -189,7 +200,7 @@ func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Stat
 	case *sqlparser.ExplainStmt:
 		return buildRoutePlan(stmt, reservedVars, vschema, buildExplainStmtPlan)
 	case *sqlparser.VExplainStmt:
-		return buildVExplainPlan(ctx, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+		return buildVExplainPlan(ctx, stmt, reservedVars, vschema, cfg)
 	case *sqlparser.OtherAdmin:
 		return buildOtherReadAndAdmin(query, vschema)
 	case *sqlparser.Analyze:

go/vt/vtgate/planbuilder/ddl.go

@@ -9,6 +9,7 @@ import (
 	vschemapb "vitess.io/vitess/go/vt/proto/vschema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -43,11 +44,11 @@ func (fk *fkContraint) FkWalk(node sqlparser.SQLNode) (kontinue bool, err error)
 // a session context. It's only when we Execute() the primitive that we have that context.
 // This is why we return a compound primitive (DDL) which contains fully populated primitives (Send & OnlineDDL),
 // and which chooses which of the two to invoke at runtime.
-func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*planResult, error) {
+func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
 	if vschema.Destination() != nil {
 		return buildByPassPlan(sql, vschema, true)
 	}
-	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(ctx, sql, ddlStatement, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(ctx, sql, ddlStatement, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -61,15 +62,12 @@ func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser
 	}
 
 	eddl := &engine.DDL{
-		Keyspace:  normalDDLPlan.Keyspace,
-		SQL:       normalDDLPlan.Query,
-		DDL:       ddlStatement,
-		NormalDDL: normalDDLPlan,
-		OnlineDDL: onlineDDLPlan,
-
-		DirectDDLEnabled: enableDirectDDL,
-		OnlineDDLEnabled: enableOnlineDDL,
-
+		Keyspace:        normalDDLPlan.Keyspace,
+		SQL:             normalDDLPlan.Query,
+		DDL:             ddlStatement,
+		NormalDDL:       normalDDLPlan,
+		OnlineDDL:       onlineDDLPlan,
+		Config:          cfg,
 		CreateTempTable: ddlStatement.IsTemporary(),
 	}
 	tc := &tableCollector{}
@@ -94,7 +92,7 @@ func buildByPassPlan(sql string, vschema plancontext.VSchema, isDDL bool) (*plan
 	return newPlanResult(send), nil
 }
 
-func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*engine.Send, *engine.OnlineDDL, error) {
+func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*engine.Send, *engine.OnlineDDL, error) {
 	var destination key.Destination
 	var keyspace *vindexes.Keyspace
 	var err error
@@ -113,9 +111,9 @@ func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLSt
 		}
 		err = checkFKError(vschema, ddlStatement, keyspace)
 	case *sqlparser.CreateView:
-		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
+		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, cfg, ddl.Select, ddl)
 	case *sqlparser.AlterView:
-		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
+		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, cfg, ddl.Select, ddl)
 	case *sqlparser.DropView:
 		destination, keyspace, err = buildDropView(vschema, ddlStatement)
 	case *sqlparser.DropTable:
@@ -197,7 +195,7 @@ func buildCreateViewCommon(
 	ctx context.Context,
 	vschema plancontext.VSchema,
 	reservedVars *sqlparser.ReservedVars,
-	enableOnlineDDL, enableDirectDDL bool,
+	cfg dynamicconfig.DDL,
 	ddlSelect sqlparser.SelectStatement,
 	ddl sqlparser.DDLStatement,
 ) (key.Destination, *vindexes.Keyspace, error) {
@@ -214,7 +212,7 @@ func buildCreateViewCommon(
 		expressions = append(expressions, sqlparser.Clone(p.SelectExprs))
 		return nil
 	})
-	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, nil, err
 	}

go/vt/vtgate/planbuilder/migration.go

@@ -27,6 +27,7 @@ import (
 	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -80,8 +81,8 @@ func buildAlterMigrationThrottleAppPlan(query string, alterMigration *sqlparser.
 	}), nil
 }
 
-func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigration, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigration, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 
@@ -118,8 +119,8 @@ func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigrat
 	return newPlanResult(send), nil
 }
 
-func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")
@@ -147,8 +148,8 @@ func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vsc
 	return newPlanResult(emig), nil
 }
 
-func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")

go/vt/vtgate/planbuilder/simplifier_test.go

@@ -135,12 +135,12 @@ func keepSameError(query string, reservedVars *sqlparser.ReservedVars, vschema *
 	}
 	rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil)
 	ast := rewritten.AST
-	_, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, true, true)
+	_, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, staticConfig{})
 	if expected == nil {
 		panic("query does not fail to plan")
 	}
 	return func(statement sqlparser.SelectStatement) bool {
-		_, myErr := BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, true, true)
+		_, myErr := BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, staticConfig{})
 		if myErr == nil {
 			return false
 		}
@@ -162,7 +162,7 @@ func keepPanicking(query string, reservedVars *sqlparser.ReservedVars, vschema *
 			}
 		}()
 		log.Errorf("trying %s", sqlparser.String(statement))
-		_, _ = BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, true, true)
+		_, _ = BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, staticConfig{})
 		log.Errorf("did not panic")
 
 		return false