placement: give default 2 followers for non-sugar syntax (#31000)

pingcap · Dec 24, 2021 · 93d28b9 · 93d28b9
1 parent 30c5f5b
commit 93d28b9
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 26 deletions.
diff --git a/ddl/placement/bundle.go b/ddl/placement/bundle.go
@@ -85,33 +85,23 @@ func NewBundleFromConstraintsOptions(options *model.PlacementSettings) (*Bundle,
 			return nil, fmt.Errorf("%w: LeaderConstraints conflicts with Constraints", err)
 		}
 	}
-	if len(LeaderConstraints) > 0 {
-		Rules = append(Rules, NewRule(Leader, 1, LeaderConstraints))
-	} else if followerCount == 0 {
-		return nil, fmt.Errorf("%w: you must at least provide common/leader constraints, or set some followers", ErrInvalidPlacementOptions)
-	}
-
-	if followerCount > 0 {
-		// if user did not specify leader, add one
-		if len(LeaderConstraints) == 0 {
-			Rules = append(Rules, NewRule(Leader, 1, NewConstraintsDirect()))
-		}
+	Rules = append(Rules, NewRule(Leader, 1, LeaderConstraints))
 
-		FollowerRules, err := NewRules(Voter, followerCount, followerConstraints)
-		if err != nil {
-			return nil, fmt.Errorf("%w: invalid FollowerConstraints", err)
-		}
-		for _, rule := range FollowerRules {
-			for _, cnst := range CommonConstraints {
-				if err := rule.Constraints.Add(cnst); err != nil {
-					return nil, fmt.Errorf("%w: FollowerConstraints conflicts with Constraints", err)
-				}
+	if followerCount == 0 {
+		followerCount = 2
+	}
+	FollowerRules, err := NewRules(Voter, followerCount, followerConstraints)
+	if err != nil {
+		return nil, fmt.Errorf("%w: invalid FollowerConstraints", err)
+	}
+	for _, rule := range FollowerRules {
+		for _, cnst := range CommonConstraints {
+			if err := rule.Constraints.Add(cnst); err != nil {
+				return nil, fmt.Errorf("%w: FollowerConstraints conflicts with Constraints", err)
 			}
 		}
-		Rules = append(Rules, FollowerRules...)
-	} else if followerConstraints != "" {
-		return nil, fmt.Errorf("%w: specify follower constraints without specify how many followers to be placed", ErrInvalidPlacementOptions)
 	}
+	Rules = append(Rules, FollowerRules...)
 
 	if learnerCount > 0 {
 		LearnerRules, err := NewRules(Learner, learnerCount, learnerConstraints)

diff --git a/ddl/placement/bundle_test.go b/ddl/placement/bundle_test.go
@@ -572,7 +572,10 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) {
 			LeaderConstraints:   "[+region=as]",
 			FollowerConstraints: "[-region=us]",
 		},
-		err: ErrInvalidPlacementOptions,
+		output: []*Rule{
+			NewRule(Leader, 1, NewConstraintsDirect(NewConstraintDirect("region", In, "as"))),
+			NewRule(Voter, 2, NewConstraintsDirect(NewConstraintDirect("region", NotIn, "us"))),
+		},
 	})
 
 	tests = append(tests, TestCase{

diff --git a/parser/parser.go b/parser/parser.go
@@ -11289,7 +11289,12 @@ yynewstate:
 		}
 	case 10:
 		{
-			parser.yyVAL.item = &ast.PlacementOption{Tp: ast.PlacementOptionFollowerCount, UintValue: yyS[yypt-0].item.(uint64)}
+			cnt := yyS[yypt-0].item.(uint64)
+			if cnt == 0 {
+				yylex.AppendError(yylex.Errorf("FOLLOWERS must be positive"))
+				return 1
+			}
+			parser.yyVAL.item = &ast.PlacementOption{Tp: ast.PlacementOptionFollowerCount, UintValue: cnt}
 		}
 	case 11:
 		{

diff --git a/parser/parser.y b/parser/parser.y
@@ -1526,7 +1526,12 @@ DirectPlacementOption:
 	}
 |	"FOLLOWERS" EqOpt LengthNum
 	{
-		$$ = &ast.PlacementOption{Tp: ast.PlacementOptionFollowerCount, UintValue: $3.(uint64)}
+		cnt := $3.(uint64)
+		if cnt == 0 {
+			yylex.AppendError(yylex.Errorf("FOLLOWERS must be positive"))
+			return 1
+		}
+		$$ = &ast.PlacementOption{Tp: ast.PlacementOptionFollowerCount, UintValue: cnt}
 	}
 |	"VOTERS" EqOpt LengthNum
 	{

diff --git a/parser/parser_test.go b/parser/parser_test.go
@@ -2338,6 +2338,7 @@ func TestDDL(t *testing.T) {
 		{`create table t (c int) regions="us,3";`, true, "CREATE TABLE `t` (`c` INT) REGIONS = 'us,3'"},
 		{`create table t (c int) followers="us,3";`, false, ""},
 		{`create table t (c int) followers=3;`, true, "CREATE TABLE `t` (`c` INT) FOLLOWERS = 3"},
+		{`create table t (c int) followers=0;`, false, ""},
 		{`create table t (c int) voters="us,3";`, false, ""},
 		{`create table t (c int) voters=3;`, true, "CREATE TABLE `t` (`c` INT) VOTERS = 3"},
 		{`create table t (c int) learners="us,3";`, false, ""},
@@ -2353,6 +2354,7 @@ func TestDDL(t *testing.T) {
 		{`create table t (c int) /*T![placement] regions="us,3" */;`, true, "CREATE TABLE `t` (`c` INT) REGIONS = 'us,3'"},
 		{`create table t (c int) /*T![placement] followers="us,3 */";`, false, ""},
 		{`create table t (c int) /*T![placement] followers=3 */;`, true, "CREATE TABLE `t` (`c` INT) FOLLOWERS = 3"},
+		{`create table t (c int) /*T![placement] followers=0 */;`, false, ""},
 		{`create table t (c int) /*T![placement] voters="us,3" */;`, false, ""},
 		{`create table t (c int) /*T![placement] primary_region="us" regions="us,3"  */;`, true, "CREATE TABLE `t` (`c` INT) PRIMARY_REGION = 'us' REGIONS = 'us,3'"},
 		{"create table t (c int) /*T![placement] placement policy=`x` */;", true, "CREATE TABLE `t` (`c` INT) PLACEMENT POLICY = `x`"},
@@ -2361,6 +2363,7 @@ func TestDDL(t *testing.T) {
 		{`alter table t primary_region="us";`, true, "ALTER TABLE `t` PRIMARY_REGION = 'us'"},
 		{`alter table t regions="us,3";`, true, "ALTER TABLE `t` REGIONS = 'us,3'"},
 		{`alter table t followers=3;`, true, "ALTER TABLE `t` FOLLOWERS = 3"},
+		{`alter table t followers=0;`, false, ""},
 		{`alter table t voters=3;`, true, "ALTER TABLE `t` VOTERS = 3"},
 		{`alter table t learners=3;`, true, "ALTER TABLE `t` LEARNERS = 3"},
 		{`alter table t schedule="even";`, true, "ALTER TABLE `t` SCHEDULE = 'even'"},
@@ -2375,6 +2378,7 @@ func TestDDL(t *testing.T) {
 		{`create database t primary_region="us";`, true, "CREATE DATABASE `t` PRIMARY_REGION = 'us'"},
 		{`create database t regions="us,3";`, true, "CREATE DATABASE `t` REGIONS = 'us,3'"},
 		{`create database t followers=3;`, true, "CREATE DATABASE `t` FOLLOWERS = 3"},
+		{`create database t followers=0;`, false, ""},
 		{`create database t voters=3;`, true, "CREATE DATABASE `t` VOTERS = 3"},
 		{`create database t learners=3;`, true, "CREATE DATABASE `t` LEARNERS = 3"},
 		{`create database t schedule="even";`, true, "CREATE DATABASE `t` SCHEDULE = 'even'"},
@@ -2390,6 +2394,7 @@ func TestDDL(t *testing.T) {
 		{`alter database t primary_region="us";`, true, "ALTER DATABASE `t` PRIMARY_REGION = 'us'"},
 		{`alter database t regions="us,3";`, true, "ALTER DATABASE `t` REGIONS = 'us,3'"},
 		{`alter database t followers=3;`, true, "ALTER DATABASE `t` FOLLOWERS = 3"},
+		{`alter database t followers=0;`, false, ""},
 		{`alter database t voters=3;`, true, "ALTER DATABASE `t` VOTERS = 3"},
 		{`alter database t learners=3;`, true, "ALTER DATABASE `t` LEARNERS = 3"},
 		{`alter database t schedule="even";`, true, "ALTER DATABASE `t` SCHEDULE = 'even'"},
@@ -3382,6 +3387,7 @@ func TestDDL(t *testing.T) {
 		{"create placement policy x primary_region='us'", true, "CREATE PLACEMENT POLICY `x` PRIMARY_REGION = 'us'"},
 		{"create placement policy x region='us, 3'", false, ""},
 		{"create placement policy x followers=3", true, "CREATE PLACEMENT POLICY `x` FOLLOWERS = 3"},
+		{"create placement policy x followers=0", false, ""},
 		{"create placement policy x voters=3", true, "CREATE PLACEMENT POLICY `x` VOTERS = 3"},
 		{"create placement policy x learners=3", true, "CREATE PLACEMENT POLICY `x` LEARNERS = 3"},
 		{"create placement policy x schedule='even'", true, "CREATE PLACEMENT POLICY `x` SCHEDULE = 'even'"},