diff --git a/go/vt/schema/ddl_strategy.go b/go/vt/schema/ddl_strategy.go index 88400d423fd..bc33c8cb3cf 100644 --- a/go/vt/schema/ddl_strategy.go +++ b/go/vt/schema/ddl_strategy.go @@ -20,6 +20,7 @@ import ( "fmt" "regexp" "strconv" + "strings" "time" "github.com/google/shlex" @@ -115,6 +116,14 @@ func ParseDDLStrategy(strategyVariable string) (*DDLStrategySetting, error) { if _, err := setting.RetainArtifactsDuration(); err != nil { return nil, err } + + switch setting.Strategy { + case DDLStrategyVitess, DDLStrategyOnline, DDLStrategyMySQL, DDLStrategyDirect: + if opts := setting.RuntimeOptions(); len(opts) > 0 { + return nil, fmt.Errorf("invalid flags for %v strategy: %s", setting.Strategy, strings.Join(opts, " ")) + } + } + return setting, nil } diff --git a/go/vt/schema/ddl_strategy_test.go b/go/vt/schema/ddl_strategy_test.go index 8ad6ff592dc..ba7d029b8b7 100644 --- a/go/vt/schema/ddl_strategy_test.go +++ b/go/vt/schema/ddl_strategy_test.go @@ -41,19 +41,23 @@ func TestIsDirect(t *testing.T) { func TestIsCutOverThresholdFlag(t *testing.T) { tt := []struct { - s string - expect bool - val string - d time.Duration + s string + expect bool + expectError string + val string + d time.Duration }{ { - s: "something", + s: "something", + expectError: "invalid flags", }, { - s: "-cut-over-threshold", + s: "-cut-over-threshold", + expectError: "invalid flags", }, { - s: "--cut-over-threshold", + s: "--cut-over-threshold", + expectError: "invalid flags", }, { s: "--cut-over-threshold=", @@ -87,6 +91,11 @@ func TestIsCutOverThresholdFlag(t *testing.T) { for _, ts := range tt { t.Run(ts.s, func(t *testing.T) { setting, err := ParseDDLStrategy("online " + ts.s) + if ts.expectError != "" { + assert.ErrorContains(t, err, ts.expectError) + return + } + assert.NoError(t, err) val, isCutOver := isCutOverThresholdFlag(ts.s) @@ -104,19 +113,23 @@ func TestIsCutOverThresholdFlag(t *testing.T) { func TestIsExpireArtifactsFlag(t *testing.T) { tt := []struct { - s string - expect bool - val string - d time.Duration + s string + expect bool + expectError string + val string + d time.Duration }{ { - s: "something", + s: "something", + expectError: "invalid flags", }, { - s: "-retain-artifacts", + s: "-retain-artifacts", + expectError: "invalid flags", }, { - s: "--retain-artifacts", + s: "--retain-artifacts", + expectError: "invalid flags", }, { s: "--retain-artifacts=", @@ -150,6 +163,10 @@ func TestIsExpireArtifactsFlag(t *testing.T) { for _, ts := range tt { t.Run(ts.s, func(t *testing.T) { setting, err := ParseDDLStrategy("online " + ts.s) + if ts.expectError != "" { + assert.ErrorContains(t, err, ts.expectError) + return + } assert.NoError(t, err) val, isRetainArtifacts := isRetainArtifactsFlag(ts.s) @@ -183,7 +200,7 @@ func TestParseDDLStrategy(t *testing.T) { cutOverThreshold time.Duration expireArtifacts time.Duration runtimeOptions string - err error + expectError string }{ { strategyVariable: "direct", @@ -317,10 +334,29 @@ func TestParseDDLStrategy(t *testing.T) { runtimeOptions: "", analyzeTable: true, }, + + { + strategyVariable: "vitess --alow-concrrnt", // intentional typo + strategy: DDLStrategyVitess, + options: "", + runtimeOptions: "", + expectError: "invalid flags", + }, + { + strategyVariable: "vitess --declarative --max-load=Threads_running=100", + strategy: DDLStrategyVitess, + options: "--declarative --max-load=Threads_running=100", + runtimeOptions: "--max-load=Threads_running=100", + expectError: "invalid flags", + }, } for _, ts := range tt { t.Run(ts.strategyVariable, func(t *testing.T) { setting, err := ParseDDLStrategy(ts.strategyVariable) + if ts.expectError != "" { + assert.ErrorContains(t, err, ts.expectError) + return + } assert.NoError(t, err) assert.Equal(t, ts.strategy, setting.Strategy) assert.Equal(t, ts.options, setting.Options)