check if sharding rule already exists

coordinator/provider/coordinator.go

@@ -376,7 +376,7 @@ func (qc *qdbCoordinator) ListShardingRules(ctx context.Context) ([]*shrule.Shar
 
 func (qc *qdbCoordinator) AddShardingRule(ctx context.Context, rule *shrule.ShardingRule) error {
 	// Store sharding rule to metadb.
-	if err := qc.db.AddShardingRule(ctx, shrule.ShardingRuleToDB(rule)); err != nil {
+	if err := ops.AddShardingRuleWithChecks(ctx, qc.db, rule); err != nil {
 		return err
 	}
 

qdb/etcdqdb.go

@@ -152,16 +152,24 @@ func (q *EtcdQDB) GetShardingRule(ctx context.Context, id string) (*ShardingRule
 	if err != nil {
 		return nil, err
 	}
-	var rule ShardingRule
-	if err := json.Unmarshal(resp.Kvs[0].Value, &rule); err != nil {
-		return nil, err
-	}
 
-	spqrlog.Zero.Debug().
-		Interface("response", resp).
-		Msg("etcdqdb: get sharding rule")
+	switch len(resp.Kvs) {
+	case 0:
+		return nil, fmt.Errorf("sharding rule %v already present in qdb", id)
+	case 1:
+		var rule ShardingRule
+		if err := json.Unmarshal(resp.Kvs[0].Value, &rule); err != nil {
+			return nil, err
+		}
+		spqrlog.Zero.Debug().
+			Interface("response", resp).
+			Msg("etcdqdb: get sharding rule")
+
+		return &rule, nil
+	default:
+		return nil, fmt.Errorf("too much sharding rules matched: %d", len(resp.Kvs))
+	}
 
-	return &rule, nil
 }
 
 func (q *EtcdQDB) ListShardingRules(ctx context.Context) ([]*ShardingRule, error) {
@@ -468,7 +476,20 @@ func (q *EtcdQDB) CheckLockedKeyRange(ctx context.Context, id string) (*KeyRange
 	spqrlog.Zero.Debug().
 		Str("id", id).
 		Msg("etcdqdb: check locked key range")
-	return nil, fmt.Errorf("implement CheckLockedKeyRange")
+
+	resp, err := q.cli.Get(ctx, keyLockPath(keyRangeNodePath(id)))
+	if err != nil {
+		return nil, err
+	}
+
+	switch len(resp.Kvs) {
+	case 0:
+		return nil, fmt.Errorf("key range %v not locked", id)
+	case 1:
+		return q.GetKeyRange(ctx, id)
+	default:
+		return nil, fmt.Errorf("too much key ranges matched: %d", len(resp.Kvs))
+	}
 }
 
 func (q *EtcdQDB) ShareKeyRange(id string) error {

qdb/memqdb.go

@@ -406,9 +406,9 @@ func (q *MemQDB) CheckLockedKeyRange(ctx context.Context, id string) (*KeyRange,
 	q.mu.RLock()
 	defer q.mu.RUnlock()
 
-	krs, ok := q.Krs[id]
-	if !ok {
-		return nil, fmt.Errorf("no such krid")
+	krs, err := q.GetKeyRange(ctx, id)
+	if err != nil {
+		return nil, err
 	}
 
 	if !q.Freq[id] {

qdb/ops/ops.go

@@ -108,7 +108,10 @@ func MatchShardingRule(ctx context.Context, mgr meta.EntityMgr, relationName str
 }
 
 func ModifyKeyRangeWithChecks(ctx context.Context, qdb qdb.QDB, keyRange *kr.KeyRange) error {
-	// TODO: check lock are properly hold while updating
+	_, err := qdb.CheckLockedKeyRange(ctx, keyRange.ID)
+	if err != nil {
+		return err
+	}
 
 	if _, err := qdb.GetShard(ctx, keyRange.ShardID); err != nil {
 		return err

test/feature/features/coordinator.feature

@@ -143,6 +143,26 @@ Feature: Coordinator test
     }]
     """
 
+  Scenario: Add key range with the same id fails
+    When I run SQL on host "coordinator"
+    """
+    ADD KEY RANGE krid1 FROM 50 TO 100 ROUTE TO sh1
+    """
+    Then SQL error on host "coordinator" should match regexp
+    """
+    key range krid1 already present in qdb
+    """
+
+  Scenario: Add sharding rule with the same id fails
+    When I run SQL on host "coordinator"
+    """
+    ADD SHARDING RULE r1 COLUMN idx
+    """
+    Then SQL error on host "coordinator" should match regexp
+    """
+    sharding rule r1 already present in qdb
+    """
+
   Scenario: Lock/Unlock key range works
     Given I run SQL on host "coordinator"
     """