Merge pull request #8116 from planetscale/vtgate-allow-ddl-flag

Support for vtgate -enable_direct_ddl flag

go/vt/schema/online_ddl.go

@@ -41,6 +41,8 @@ var (
 )
 
 var (
+	// ErrDirectDDLDisabled is returned when direct DDL is disabled, and a user attempts to run a DDL statement
+	ErrDirectDDLDisabled = errors.New("direct DDL is disabled")
 	// ErrOnlineDDLDisabled is returned when online DDL is disabled, and a user attempts to run an online DDL operation (submit, review, control)
 	ErrOnlineDDLDisabled = errors.New("online DDL is disabled")
 	ErrForeignKeyFound   = errors.New("Foreign key found")

go/vt/vtgate/engine/ddl.go

@@ -37,6 +37,7 @@ type DDL struct {
 	NormalDDL *Send
 	OnlineDDL *OnlineDDL
 
+	DirectDDLEnabled bool
 	OnlineDDLEnabled bool
 
 	CreateTempTable bool
@@ -98,14 +99,18 @@ func (ddl *DDL) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable
 	}
 	ddl.OnlineDDL.DDLStrategySetting = ddlStrategySetting
 
-	if ddl.isOnlineSchemaDDL() {
+	switch {
+	case ddl.isOnlineSchemaDDL():
 		if !ddl.OnlineDDLEnabled {
 			return nil, schema.ErrOnlineDDLDisabled
 		}
 		return ddl.OnlineDDL.Execute(vcursor, bindVars, wantfields)
+	default: // non online-ddl
+		if !ddl.DirectDDLEnabled {
+			return nil, schema.ErrDirectDDLDisabled
+		}
+		return ddl.NormalDDL.Execute(vcursor, bindVars, wantfields)
 	}
-
-	return ddl.NormalDDL.Execute(vcursor, bindVars, wantfields)
 }
 
 // StreamExecute implements the Primitive interface

go/vt/vtgate/executor.go

@@ -1232,7 +1232,7 @@ func (e *Executor) getPlan(vcursor *vcursorImpl, sql string, comments sqlparser.
 		return plan.(*engine.Plan), nil
 	}
 
-	plan, err := planbuilder.BuildFromStmt(query, statement, reservedVars, vcursor, bindVarNeeds)
+	plan, err := planbuilder.BuildFromStmt(query, statement, reservedVars, vcursor, bindVarNeeds, *enableOnlineDDL, *enableDirectDDL)
 	if err != nil {
 		return nil, err
 	}

go/vt/vtgate/executor_ddl_test.go

@@ -0,0 +1,70 @@
+/*
+Copyright 2021 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 vtgate
+
+import (
+	"fmt"
+	"testing"
+
+	vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestDDLFlags(t *testing.T) {
+	executor, _, _, _ := createLegacyExecutorEnv()
+	session := NewSafeSession(&vtgatepb.Session{TargetString: KsTestUnsharded})
+	defer func() {
+		*enableOnlineDDL = true
+		*enableDirectDDL = true
+	}()
+	testcases := []struct {
+		enableDirectDDL bool
+		enableOnlineDDL bool
+		sql             string
+		wantErr         bool
+		err             string
+	}{
+		{
+			enableDirectDDL: false,
+			sql:             "create table t (id int)",
+			wantErr:         true,
+			err:             "direct DDL is disabled",
+		}, {
+			enableDirectDDL: true,
+			sql:             "create table t (id int)",
+			wantErr:         false,
+		}, {
+			enableOnlineDDL: false,
+			sql:             "revert vitess_migration 'abc'",
+			wantErr:         true,
+			err:             "online DDL is disabled",
+		},
+	}
+	for _, testcase := range testcases {
+		t.Run(fmt.Sprintf("%s-%v-%v", testcase.sql, testcase.enableDirectDDL, testcase.enableOnlineDDL), func(t *testing.T) {
+			*enableDirectDDL = testcase.enableDirectDDL
+			*enableOnlineDDL = testcase.enableOnlineDDL
+			_, err := executor.Execute(ctx, "TestDDLFlags", session, testcase.sql, nil)
+			if testcase.wantErr {
+				require.EqualError(t, err, testcase.err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

go/vt/vtgate/planbuilder/builder.go

@@ -18,7 +18,6 @@ package planbuilder
 
 import (
 	"errors"
-	"flag"
 	"sort"
 
 	"vitess.io/vitess/go/sqltypes"
@@ -36,10 +35,6 @@ import (
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 )
 
-var (
-	enableOnlineDDL = flag.Bool("enable_online_ddl", true, "Allow users to submit, review and control Online DDL")
-)
-
 // ContextVSchema defines the interface for this package to fetch
 // info about tables.
 type ContextVSchema interface {
@@ -104,15 +99,15 @@ func TestBuilder(query string, vschema ContextVSchema) (*engine.Plan, error) {
 	}
 
 	reservedVars := sqlparser.NewReservedVars("vtg", reserved)
-	return BuildFromStmt(query, result.AST, reservedVars, vschema, result.BindVarNeeds)
+	return BuildFromStmt(query, result.AST, reservedVars, vschema, result.BindVarNeeds, true, true)
 }
 
 // ErrPlanNotSupported is an error for plan building not supported
 var ErrPlanNotSupported = errors.New("plan building not supported")
 
 // BuildFromStmt builds a plan based on the AST provided.
-func BuildFromStmt(query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, bindVarNeeds *sqlparser.BindVarNeeds) (*engine.Plan, error) {
-	instruction, err := createInstructionFor(query, stmt, reservedVars, vschema)
+func BuildFromStmt(query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, bindVarNeeds *sqlparser.BindVarNeeds, enableOnlineDDL, enableDirectDDL bool) (*engine.Plan, error) {
+	instruction, err := createInstructionFor(query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, err
 	}
@@ -150,7 +145,7 @@ func buildRoutePlan(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVa
 
 type selectPlanner func(query string) func(sqlparser.Statement, *sqlparser.ReservedVars, ContextVSchema) (engine.Primitive, error)
 
-func createInstructionFor(query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (engine.Primitive, error) {
+func createInstructionFor(query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, enableOnlineDDL, enableDirectDDL bool) (engine.Primitive, error) {
 	switch stmt := stmt.(type) {
 	case *sqlparser.Select:
 		configuredPlanner, err := getConfiguredPlanner(vschema)
@@ -167,17 +162,17 @@ func createInstructionFor(query string, stmt sqlparser.Statement, reservedVars *
 	case *sqlparser.Union:
 		return buildRoutePlan(stmt, reservedVars, vschema, buildUnionPlan)
 	case sqlparser.DDLStatement:
-		return buildGeneralDDLPlan(query, stmt, reservedVars, vschema)
+		return buildGeneralDDLPlan(query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	case *sqlparser.AlterMigration:
-		return buildAlterMigrationPlan(query, vschema)
+		return buildAlterMigrationPlan(query, vschema, enableOnlineDDL)
 	case *sqlparser.RevertMigration:
-		return buildRevertMigrationPlan(query, stmt, vschema)
+		return buildRevertMigrationPlan(query, stmt, vschema, enableOnlineDDL)
 	case *sqlparser.AlterVschema:
 		return buildVSchemaDDLPlan(stmt, vschema)
 	case *sqlparser.Use:
 		return buildUsePlan(stmt, vschema)
 	case sqlparser.Explain:
-		return buildExplainPlan(stmt, reservedVars, vschema)
+		return buildExplainPlan(stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	case *sqlparser.OtherRead, *sqlparser.OtherAdmin:
 		return buildOtherReadAndAdmin(query, vschema)
 	case *sqlparser.Set:

go/vt/vtgate/planbuilder/ddl.go

@@ -50,11 +50,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(sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (engine.Primitive, error) {
+func buildGeneralDDLPlan(sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, enableOnlineDDL, enableDirectDDL bool) (engine.Primitive, error) {
 	if vschema.Destination() != nil {
 		return buildByPassDDLPlan(sql, vschema)
 	}
-	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(sql, ddlStatement, reservedVars, vschema)
+	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(sql, ddlStatement, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, err
 	}
@@ -68,12 +68,14 @@ func buildGeneralDDLPlan(sql string, ddlStatement sqlparser.DDLStatement, reserv
 	}
 
 	return &engine.DDL{
-		Keyspace:         normalDDLPlan.Keyspace,
-		SQL:              normalDDLPlan.Query,
-		DDL:              ddlStatement,
-		NormalDDL:        normalDDLPlan,
-		OnlineDDL:        onlineDDLPlan,
-		OnlineDDLEnabled: *enableOnlineDDL,
+		Keyspace:  normalDDLPlan.Keyspace,
+		SQL:       normalDDLPlan.Query,
+		DDL:       ddlStatement,
+		NormalDDL: normalDDLPlan,
+		OnlineDDL: onlineDDLPlan,
+
+		DirectDDLEnabled: enableDirectDDL,
+		OnlineDDLEnabled: enableOnlineDDL,
 
 		CreateTempTable: ddlStatement.IsTemporary(),
 	}, nil
@@ -91,7 +93,7 @@ func buildByPassDDLPlan(sql string, vschema ContextVSchema) (engine.Primitive, e
 	}, nil
 }
 
-func buildDDLPlans(sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (*engine.Send, *engine.OnlineDDL, error) {
+func buildDDLPlans(sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, enableOnlineDDL, enableDirectDDL bool) (*engine.Send, *engine.OnlineDDL, error) {
 	var destination key.Destination
 	var keyspace *vindexes.Keyspace
 	var err error
@@ -106,9 +108,9 @@ func buildDDLPlans(sql string, ddlStatement sqlparser.DDLStatement, reservedVars
 		// We should find the target of the query from this tables location
 		destination, keyspace, err = findTableDestinationAndKeyspace(vschema, ddlStatement)
 	case *sqlparser.CreateView:
-		destination, keyspace, err = buildCreateView(vschema, ddl, reservedVars)
+		destination, keyspace, err = buildCreateView(vschema, ddl, reservedVars, enableOnlineDDL, enableDirectDDL)
 	case *sqlparser.AlterView:
-		destination, keyspace, err = buildAlterView(vschema, ddl, reservedVars)
+		destination, keyspace, err = buildAlterView(vschema, ddl, reservedVars, enableOnlineDDL, enableDirectDDL)
 	case *sqlparser.CreateTable:
 		err = checkFKError(vschema, ddlStatement)
 		if err != nil {
@@ -192,7 +194,7 @@ func findTableDestinationAndKeyspace(vschema ContextVSchema, ddlStatement sqlpar
 	return destination, keyspace, nil
 }
 
-func buildAlterView(vschema ContextVSchema, ddl *sqlparser.AlterView, reservedVars *sqlparser.ReservedVars) (key.Destination, *vindexes.Keyspace, error) {
+func buildAlterView(vschema ContextVSchema, ddl *sqlparser.AlterView, reservedVars *sqlparser.ReservedVars, enableOnlineDDL, enableDirectDDL bool) (key.Destination, *vindexes.Keyspace, error) {
 	// For Alter View, we require that the view exist and the select query can be satisfied within the keyspace itself
 	// We should remove the keyspace name from the table name, as the database name in MySQL might be different than the keyspace name
 	destination, keyspace, err := findTableDestinationAndKeyspace(vschema, ddl)
@@ -201,7 +203,7 @@ func buildAlterView(vschema ContextVSchema, ddl *sqlparser.AlterView, reservedVa
 	}
 
 	var selectPlan engine.Primitive
-	selectPlan, err = createInstructionFor(sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema)
+	selectPlan, err = createInstructionFor(sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -227,7 +229,7 @@ func buildAlterView(vschema ContextVSchema, ddl *sqlparser.AlterView, reservedVa
 	return destination, keyspace, nil
 }
 
-func buildCreateView(vschema ContextVSchema, ddl *sqlparser.CreateView, reservedVars *sqlparser.ReservedVars) (key.Destination, *vindexes.Keyspace, error) {
+func buildCreateView(vschema ContextVSchema, ddl *sqlparser.CreateView, reservedVars *sqlparser.ReservedVars, enableOnlineDDL, enableDirectDDL bool) (key.Destination, *vindexes.Keyspace, error) {
 	// For Create View, we require that the keyspace exist and the select query can be satisfied within the keyspace itself
 	// We should remove the keyspace name from the table name, as the database name in MySQL might be different than the keyspace name
 	destination, keyspace, _, err := vschema.TargetDestination(ddl.ViewName.Qualifier.String())
@@ -237,7 +239,7 @@ func buildCreateView(vschema ContextVSchema, ddl *sqlparser.CreateView, reserved
 	ddl.ViewName.Qualifier = sqlparser.NewTableIdent("")
 
 	var selectPlan engine.Primitive
-	selectPlan, err = createInstructionFor(sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema)
+	selectPlan, err = createInstructionFor(sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, nil, err
 	}

go/vt/vtgate/planbuilder/explain.go

@@ -29,13 +29,13 @@ import (
 )
 
 // Builds an explain-plan for the given Primitive
-func buildExplainPlan(stmt sqlparser.Explain, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (engine.Primitive, error) {
+func buildExplainPlan(stmt sqlparser.Explain, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, enableOnlineDDL, enableDirectDDL bool) (engine.Primitive, error) {
 	switch explain := stmt.(type) {
 	case *sqlparser.ExplainTab:
 		return explainTabPlan(explain, vschema)
 	case *sqlparser.ExplainStmt:
 		if explain.Type == sqlparser.VitessType {
-			return buildVitessTypePlan(explain, reservedVars, vschema)
+			return buildVitessTypePlan(explain, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 		}
 		return buildOtherReadAndAdmin(sqlparser.String(explain), vschema)
 	}
@@ -61,8 +61,8 @@ func explainTabPlan(explain *sqlparser.ExplainTab, vschema ContextVSchema) (engi
 	}, nil
 }
 
-func buildVitessTypePlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (engine.Primitive, error) {
-	innerInstruction, err := createInstructionFor(sqlparser.String(explain.Statement), explain.Statement, reservedVars, vschema)
+func buildVitessTypePlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema, enableOnlineDDL, enableDirectDDL bool) (engine.Primitive, error) {
+	innerInstruction, err := createInstructionFor(sqlparser.String(explain.Statement), explain.Statement, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, err
 	}

go/vt/vtgate/planbuilder/migration.go

@@ -26,8 +26,8 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/engine"
 )
 
-func buildAlterMigrationPlan(query string, vschema ContextVSchema) (engine.Primitive, error) {
-	if !*enableOnlineDDL {
+func buildAlterMigrationPlan(query string, vschema ContextVSchema, enableOnlineDDL bool) (engine.Primitive, error) {
+	if !enableOnlineDDL {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")
@@ -53,8 +53,8 @@ func buildAlterMigrationPlan(query string, vschema ContextVSchema) (engine.Primi
 	}, nil
 }
 
-func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema ContextVSchema) (engine.Primitive, error) {
-	if !*enableOnlineDDL {
+func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema ContextVSchema, enableOnlineDDL bool) (engine.Primitive, error) {
+	if !enableOnlineDDL {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")

go/vt/vtgate/vtgate.go

@@ -84,6 +84,10 @@ var (
 	warnShardedOnly   = flag.Bool("warn_sharded_only", false, "If any features that are only available in unsharded mode are used, query execution warnings will be added to the session")
 
 	foreignKeyMode = flag.String("foreign_key_mode", "allow", "This is to provide how to handle foreign key constraint in create/alter table. Valid values are: allow, disallow")
+
+	// flags to enable/disable online and direct DDL statements
+	enableOnlineDDL = flag.Bool("enable_online_ddl", true, "Allow users to submit, review and control Online DDL")
+	enableDirectDDL = flag.Bool("enable_direct_ddl", true, "Allow users to submit direct DDL statements")
 )
 
 func getTxMode() vtgatepb.TransactionMode {