From 7212bfce83e5ea50699412630c598df7aaa4211d Mon Sep 17 00:00:00 2001 From: linearb Date: Mon, 21 Sep 2015 14:59:12 -0400 Subject: [PATCH 1/7] add RENAME DATABASE --- cmd/influxd/run/server_test.go | 31 +++++++++++++-- influxql/ast.go | 25 ++++++++++++ influxql/parser.go | 47 +++++++++++++++++++++- influxql/parser_test.go | 4 +- influxql/scanner_test.go | 1 + influxql/token.go | 2 + meta/data.go | 25 ++++++++++++ meta/data_test.go | 30 ++++++++++++++ meta/internal/meta.pb.go | 37 +++++++++++++++++ meta/internal/meta.proto | 9 +++++ meta/statement_executor.go | 7 ++++ meta/statement_executor_test.go | 25 ++++++++++++ meta/store.go | 26 ++++++++++++ meta/store_test.go | 70 +++++++++++++++++++++++++++++++++ 14 files changed, 333 insertions(+), 6 deletions(-) diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index caed3a840c..d24be1587d 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -66,18 +66,43 @@ func TestServer_DatabaseCommands(t *testing.T) { command: `SHOW DATABASES`, exp: `{"results":[{"series":[{"name":"databases","columns":["name"],"values":[["db0"],["db1"]]}]}]}`, }, + &Query{ + name: "rename database should succeed", + command: `RENAME DATABASE db1 TO db2`, + exp: `{"results":[{}]}`, + }, + &Query{ + name: "show databases should reflect change of name", + command: `SHOW DATABASES`, + exp: `{"results":[{"series":[{"name":"databases","columns":["name"],"values":[["db0"],["db2"]]}]}]}`, + }, + &Query{ + name: "rename non-existent database should fail", + command: `RENAME DATABASE db4 TO db5`, + exp: `{"results":[{"error":"database not found"}]}`, + }, + &Query{ + name: "rename database to illegal name should fail", + command: `RENAME DATABASE db2 TO 0xdb0`, + exp: `{"error":"error parsing query: found 0, expected identifier at line 1, char 24"}`, + }, + &Query{ + name: "rename database to already existing datbase should fail", + command: `RENAME DATABASE db2 TO db0`, + exp: `{"results":[{"error":"database already exists"}]}`, + }, &Query{ name: "drop database db0 should succeed", command: `DROP DATABASE db0`, exp: `{"results":[{}]}`, }, &Query{ - name: "drop database db1 should succeed", - command: `DROP DATABASE db1`, + name: "drop database db2 should succeed", + command: `DROP DATABASE db2`, exp: `{"results":[{}]}`, }, &Query{ - name: "show database should have no results", + name: "show databases should have no results after dropping all databases", command: `SHOW DATABASES`, exp: `{"results":[{"series":[{"name":"databases","columns":["name"]}]}]}`, }, diff --git a/influxql/ast.go b/influxql/ast.go index 2e2d1fc8cd..75b439c7a1 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -96,6 +96,7 @@ func (*DropServerStatement) node() {} func (*DropUserStatement) node() {} func (*GrantStatement) node() {} func (*GrantAdminStatement) node() {} +func (*RenameDatabaseStatement) node() {} func (*RevokeStatement) node() {} func (*RevokeAdminStatement) node() {} func (*SelectStatement) node() {} @@ -203,6 +204,7 @@ func (*DropServerStatement) stmt() {} func (*DropUserStatement) stmt() {} func (*GrantStatement) stmt() {} func (*GrantAdminStatement) stmt() {} +func (*RenameDatabaseStatement) stmt() {} func (*ShowContinuousQueriesStatement) stmt() {} func (*ShowGrantsForUserStatement) stmt() {} func (*ShowServersStatement) stmt() {} @@ -502,6 +504,29 @@ func (s *GrantAdminStatement) RequiredPrivileges() ExecutionPrivileges { return ExecutionPrivileges{{Admin: true, Name: "", Privilege: AllPrivileges}} } +// RenameDatabaseStatement represents a command for renaming a database. +type RenameDatabaseStatement struct { + // Current name of the database + OldName string + // New name of the database + NewName string +} + +// String returns a string representation of the rename database statement. +func (s *RenameDatabaseStatement) String() string { + var buf bytes.Buffer + _, _ = buf.WriteString("RENAME DATABASE ") + _, _ = buf.WriteString(s.OldName) + _, _ = buf.WriteString(" TO ") + _, _ = buf.WriteString(s.NewName) + return buf.String() +} + +// RequiredPrivileges returns the privilege required to execute a RenameDatabaseStatement. +func (s *RenameDatabaseStatement) RequiredPrivileges() ExecutionPrivileges { + return ExecutionPrivileges{{Admin: true, Name: "", Privilege: AllPrivileges}} +} + // SetPasswordUserStatement represents a command for changing user password. type SetPasswordUserStatement struct { // Plain Password diff --git a/influxql/parser.go b/influxql/parser.go index f7b801dcb7..482582d041 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -90,6 +90,8 @@ func (p *Parser) ParseStatement() (Statement, error) { return p.parseDropStatement() case GRANT: return p.parseGrantStatement() + case RENAME: + return p.parseRenameStatement() case REVOKE: return p.parseRevokeStatement() case ALTER: @@ -97,7 +99,7 @@ func (p *Parser) ParseStatement() (Statement, error) { case SET: return p.parseSetPasswordUserStatement() default: - return nil, newParseError(tokstr(tok, lit), []string{"SELECT", "DELETE", "SHOW", "CREATE", "DROP", "GRANT", "REVOKE", "ALTER", "SET"}, pos) + return nil, newParseError(tokstr(tok, lit), []string{"SELECT", "DELETE", "SHOW", "CREATE", "DROP", "GRANT", "RENAME", "REVOKE", "ALTER", "SET"}, pos) } } @@ -229,6 +231,19 @@ func (p *Parser) parseAlterStatement() (Statement, error) { return nil, newParseError(tokstr(tok, lit), []string{"RETENTION"}, pos) } +// parseRenameStatement parses a string and returns a rename statement. +// This function assumes the RENAME token has already been consumed. +func (p *Parser) parseRenameStatement() (Statement, error) { + tok, pos, lit := p.scanIgnoreWhitespace() + if tok == DATABASE { + return p.parseRenameDatabaseStatement() + } + + return nil, newParseError(tokstr(tok, lit), []string{"DATABASE"}, pos) +} + +// parseDropStatement parses a string and returns a drop statement. + // parseSetPasswordUserStatement parses a string and returns a set statement. // This function assumes the SET token has already been consumed. func (p *Parser) parseSetPasswordUserStatement() (*SetPasswordUserStatement, error) { @@ -1352,6 +1367,36 @@ func (p *Parser) parseDropDatabaseStatement() (*DropDatabaseStatement, error) { return stmt, nil } +// parseRenameDatabaseStatement parses a string and returns a RenameDatabaseStatement. +// This function assumes the "RENAME DATABASE" tokens have already been consumed. +func (p *Parser) parseRenameDatabaseStatement() (*RenameDatabaseStatement, error) { + stmt := &RenameDatabaseStatement{} + + // Parse the name of the database to be renamed. + lit, err := p.parseIdent() + if err != nil { + return nil, err + } + stmt.OldName = lit + + // Parse TO clause. + tok, pos, lit := p.scanIgnoreWhitespace() + + // Check for required TO token. + if tok != TO { + return nil, newParseError(tokstr(tok, lit), []string{"TO"}, pos) + } + + // Parse the new name of the database. + lit, err = p.parseIdent() + if err != nil { + return nil, err + } + stmt.NewName = lit + + return stmt, nil +} + // parseDropRetentionPolicyStatement parses a string and returns a DropRetentionPolicyStatement. // This function assumes the DROP RETENTION POLICY tokens have been consumed. func (p *Parser) parseDropRetentionPolicyStatement() (*DropRetentionPolicyStatement, error) { diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 57a2c934d2..71ea830285 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -1451,10 +1451,10 @@ func TestParser_ParseStatement(t *testing.T) { }, // Errors - {s: ``, err: `found EOF, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, REVOKE, ALTER, SET at line 1, char 1`}, + {s: ``, err: `found EOF, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, RENAME, REVOKE, ALTER, SET at line 1, char 1`}, {s: `SELECT`, err: `found EOF, expected identifier, string, number, bool at line 1, char 8`}, {s: `SELECT time FROM myseries`, err: `at least 1 non-time field must be queried`}, - {s: `blah blah`, err: `found blah, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, REVOKE, ALTER, SET at line 1, char 1`}, + {s: `blah blah`, err: `found blah, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, RENAME, REVOKE, ALTER, SET at line 1, char 1`}, {s: `SELECT field1 X`, err: `found X, expected FROM at line 1, char 15`}, {s: `SELECT field1 FROM "series" WHERE X +;`, err: `found ;, expected identifier, string, number, bool at line 1, char 38`}, {s: `SELECT field1 FROM myseries GROUP`, err: `found EOF, expected BY at line 1, char 35`}, diff --git a/influxql/scanner_test.go b/influxql/scanner_test.go index 7778fc4383..1c41ae9bc4 100644 --- a/influxql/scanner_test.go +++ b/influxql/scanner_test.go @@ -150,6 +150,7 @@ func TestScanner_Scan(t *testing.T) { {s: `QUERIES`, tok: influxql.QUERIES}, {s: `QUERY`, tok: influxql.QUERY}, {s: `READ`, tok: influxql.READ}, + {s: `RENAME`, tok: influxql.RENAME}, {s: `RETENTION`, tok: influxql.RETENTION}, {s: `REVOKE`, tok: influxql.REVOKE}, {s: `SELECT`, tok: influxql.SELECT}, diff --git a/influxql/token.go b/influxql/token.go index fe521f4eba..87cc36d8f3 100644 --- a/influxql/token.go +++ b/influxql/token.go @@ -104,6 +104,7 @@ const ( QUERIES QUERY READ + RENAME REPLICATION RETENTION REVOKE @@ -216,6 +217,7 @@ var tokens = [...]string{ QUERIES: "QUERIES", QUERY: "QUERY", READ: "READ", + RENAME: "RENAME", REPLICATION: "REPLICATION", RETENTION: "RETENTION", REVOKE: "REVOKE", diff --git a/meta/data.go b/meta/data.go index 9257e61cb8..647a5fbdbc 100644 --- a/meta/data.go +++ b/meta/data.go @@ -177,6 +177,31 @@ func (data *Data) DropDatabase(name string) error { return ErrDatabaseNotFound } +// RenameDatabase renames a database. +// Returns an error if oldName or newName is blank +// or if a database with the newName already exists +// or if a database with oldName does not exist +func (data *Data) RenameDatabase(oldName, newName string) error { + if newName == "" || oldName == "" { + return ErrDatabaseNameRequired + } + if data.Database(newName) != nil { + return ErrDatabaseExists + } + if data.Database(oldName) == nil { + return ErrDatabaseNotFound + } + // find database named oldName and rename it to newName + for i := range data.Databases { + if data.Databases[i].Name == oldName { + data.Databases[i].Name = newName + //TODO CQs + return nil + } + } + return ErrDatabaseNotFound +} + // RetentionPolicy returns a retention policy for a database by name. func (data *Data) RetentionPolicy(database, name string) (*RetentionPolicyInfo, error) { di := data.Database(database) diff --git a/meta/data_test.go b/meta/data_test.go index 8b7fcc6abf..482d22c321 100644 --- a/meta/data_test.go +++ b/meta/data_test.go @@ -135,6 +135,36 @@ func TestData_DropDatabase(t *testing.T) { } } +// Ensure a database can be renamed. +func TestData_RenameDatabase(t *testing.T) { + var data meta.Data + for i := 0; i < 2; i++ { + if err := data.CreateDatabase(fmt.Sprintf("db%d", i)); err != nil { + t.Fatal(err) + } + } + + if err := data.RenameDatabase("db1", "db2"); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(data.Databases, []meta.DatabaseInfo{{Name: "db0"}, {Name: "db2"}}) { + t.Fatalf("unexpected databases: %#v", data.Databases) + } +} + +// Ensure that renaming a database without both old and new names returns an error. +func TestData_RenameDatabase_ErrNameRequired(t *testing.T) { + var data meta.Data + if err := data.RenameDatabase("", ""); err != meta.ErrDatabaseNameRequired { + t.Fatalf("unexpected error: %s", err) + } + if err := data.RenameDatabase("from_foo", ""); err != meta.ErrDatabaseNameRequired { + t.Fatalf("unexpected error: %s", err) + } + if err := data.RenameDatabase("", "to_foo"); err != meta.ErrDatabaseNameRequired { + t.Fatalf("unexpected error: %s", err) + } +} + // Ensure a retention policy can be created. func TestData_CreateRetentionPolicy(t *testing.T) { data := meta.Data{Nodes: []meta.NodeInfo{{ID: 1}, {ID: 2}}} diff --git a/meta/internal/meta.pb.go b/meta/internal/meta.pb.go index 285ccb2e41..e364a1a677 100644 --- a/meta/internal/meta.pb.go +++ b/meta/internal/meta.pb.go @@ -39,6 +39,7 @@ It has these top-level messages: SetDataCommand SetAdminPrivilegeCommand UpdateNodeCommand + RenameDatabaseCommand Response ResponseHeader ErrorResponse @@ -116,6 +117,7 @@ const ( Command_SetDataCommand Command_Type = 17 Command_SetAdminPrivilegeCommand Command_Type = 18 Command_UpdateNodeCommand Command_Type = 19 + Command_RenameDatabaseCommand Command_Type = 20 ) var Command_Type_name = map[int32]string{ @@ -138,6 +140,7 @@ var Command_Type_name = map[int32]string{ 17: "SetDataCommand", 18: "SetAdminPrivilegeCommand", 19: "UpdateNodeCommand", + 20: "RenameDatabaseCommand", } var Command_Type_value = map[string]int32{ "CreateNodeCommand": 1, @@ -159,6 +162,7 @@ var Command_Type_value = map[string]int32{ "SetDataCommand": 17, "SetAdminPrivilegeCommand": 18, "UpdateNodeCommand": 19, + "RenameDatabaseCommand": 20, } func (x Command_Type) Enum() *Command_Type { @@ -1225,6 +1229,38 @@ var E_UpdateNodeCommand_Command = &proto.ExtensionDesc{ Tag: "bytes,119,opt,name=command", } +type RenameDatabaseCommand struct { + OldName *string `protobuf:"bytes,1,req,name=oldName" json:"oldName,omitempty"` + NewName *string `protobuf:"bytes,2,req,name=newName" json:"newName,omitempty"` + XXX_unrecognized []byte `json:"-"` +} + +func (m *RenameDatabaseCommand) Reset() { *m = RenameDatabaseCommand{} } +func (m *RenameDatabaseCommand) String() string { return proto.CompactTextString(m) } +func (*RenameDatabaseCommand) ProtoMessage() {} + +func (m *RenameDatabaseCommand) GetOldName() string { + if m != nil && m.OldName != nil { + return *m.OldName + } + return "" +} + +func (m *RenameDatabaseCommand) GetNewName() string { + if m != nil && m.NewName != nil { + return *m.NewName + } + return "" +} + +var E_RenameDatabaseCommand_Command = &proto.ExtensionDesc{ + ExtendedType: (*Command)(nil), + ExtensionType: (*RenameDatabaseCommand)(nil), + Field: 120, + Name: "internal.RenameDatabaseCommand.command", + Tag: "bytes,120,opt,name=command", +} + type Response struct { OK *bool `protobuf:"varint,1,req,name=OK" json:"OK,omitempty"` Error *string `protobuf:"bytes,2,opt,name=Error" json:"Error,omitempty"` @@ -1453,4 +1489,5 @@ func init() { proto.RegisterExtension(E_SetDataCommand_Command) proto.RegisterExtension(E_SetAdminPrivilegeCommand_Command) proto.RegisterExtension(E_UpdateNodeCommand_Command) + proto.RegisterExtension(E_RenameDatabaseCommand_Command) } diff --git a/meta/internal/meta.proto b/meta/internal/meta.proto index 1dc1d76fd0..178d5057d1 100644 --- a/meta/internal/meta.proto +++ b/meta/internal/meta.proto @@ -105,6 +105,7 @@ message Command { SetDataCommand = 17; SetAdminPrivilegeCommand = 18; UpdateNodeCommand = 19; + RenameDatabaseCommand = 20; } required Type type = 1; @@ -266,6 +267,14 @@ message UpdateNodeCommand { required string Host = 2; } +message RenameDatabaseCommand { + extend Command { + optional RenameDatabaseCommand command = 120; + } + required string oldName = 1; + required string newName = 2; +} + message Response { required bool OK = 1; optional string Error = 2; diff --git a/meta/statement_executor.go b/meta/statement_executor.go index af4432006c..7899aa0c62 100644 --- a/meta/statement_executor.go +++ b/meta/statement_executor.go @@ -23,6 +23,7 @@ type StatementExecutor struct { Databases() ([]DatabaseInfo, error) CreateDatabase(name string) (*DatabaseInfo, error) DropDatabase(name string) error + RenameDatabase(oldName, newName string) error DefaultRetentionPolicy(database string) (*RetentionPolicyInfo, error) CreateRetentionPolicy(database string, rpi *RetentionPolicyInfo) (*RetentionPolicyInfo, error) @@ -69,6 +70,8 @@ func (e *StatementExecutor) ExecuteStatement(stmt influxql.Statement) *influxql. return e.executeGrantStatement(stmt) case *influxql.GrantAdminStatement: return e.executeGrantAdminStatement(stmt) + case *influxql.RenameDatabaseStatement: + return e.executeRenameDatabaseStatement(stmt) case *influxql.RevokeStatement: return e.executeRevokeStatement(stmt) case *influxql.RevokeAdminStatement: @@ -212,6 +215,10 @@ func (e *StatementExecutor) executeGrantAdminStatement(stmt *influxql.GrantAdmin return &influxql.Result{Err: e.Store.SetAdminPrivilege(stmt.User, true)} } +func (e *StatementExecutor) executeRenameDatabaseStatement(q *influxql.RenameDatabaseStatement) *influxql.Result { + return &influxql.Result{Err: e.Store.RenameDatabase(q.OldName, q.NewName)} +} + func (e *StatementExecutor) executeRevokeStatement(stmt *influxql.RevokeStatement) *influxql.Result { priv := influxql.NoPrivileges diff --git a/meta/statement_executor_test.go b/meta/statement_executor_test.go index 9aef1618a8..713cd1af5d 100644 --- a/meta/statement_executor_test.go +++ b/meta/statement_executor_test.go @@ -46,6 +46,26 @@ func TestStatementExecutor_ExecuteStatement_DropDatabase(t *testing.T) { } } +// Ensure a RENAME DATABASE statement can be executed. +func TestStatementExecutor_ExecuteStatement_RenameDatabase(t *testing.T) { + e := NewStatementExecutor() + e.Store.RenameDatabaseFn = func(oldName, newName string) error { + if oldName != "old_foo" { + t.Fatalf("unexpected name: %s", oldName) + } + if newName != "new_foo" { + t.Fatalf("unexpected name: %s", newName) + } + return nil + } + + if res := e.ExecuteStatement(influxql.MustParseStatement(`RENAME DATABASE old_foo to new_foo`)); res.Err != nil { + t.Fatal(res.Err) + } else if res.Series != nil { + t.Fatalf("unexpected rows: %#v", res.Series) + } +} + // Ensure a SHOW DATABASES statement can be executed. func TestStatementExecutor_ExecuteStatement_ShowDatabases(t *testing.T) { e := NewStatementExecutor() @@ -883,6 +903,7 @@ type StatementExecutorStore struct { CreateDatabaseFn func(name string) (*meta.DatabaseInfo, error) DropDatabaseFn func(name string) error DeleteNodeFn func(nodeID uint64, force bool) error + RenameDatabaseFn func(oldName, newName string) error DefaultRetentionPolicyFn func(database string) (*meta.RetentionPolicyInfo, error) CreateRetentionPolicyFn func(database string, rpi *meta.RetentionPolicyInfo) (*meta.RetentionPolicyInfo, error) UpdateRetentionPolicyFn func(database, name string, rpu *meta.RetentionPolicyUpdate) error @@ -940,6 +961,10 @@ func (s *StatementExecutorStore) DropDatabase(name string) error { return s.DropDatabaseFn(name) } +func (s *StatementExecutorStore) RenameDatabase(oldName, newName string) error { + return s.RenameDatabaseFn(oldName, newName) +} + func (s *StatementExecutorStore) DefaultRetentionPolicy(database string) (*meta.RetentionPolicyInfo, error) { return s.DefaultRetentionPolicyFn(database) } diff --git a/meta/store.go b/meta/store.go index 1d8da401c7..5ac07adb15 100644 --- a/meta/store.go +++ b/meta/store.go @@ -927,6 +927,16 @@ func (s *Store) DropDatabase(name string) error { ) } +// RenameDatabase renames a database in the metastore +func (s *Store) RenameDatabase(oldName, newName string) error { + return s.exec(internal.Command_RenameDatabaseCommand, internal.E_RenameDatabaseCommand_Command, + &internal.RenameDatabaseCommand{ + OldName: proto.String(oldName), + NewName: proto.String(newName), + }, + ) +} + // RetentionPolicy returns a retention policy for a database by name. func (s *Store) RetentionPolicy(database, name string) (rpi *RetentionPolicyInfo, err error) { err = s.read(func(data *Data) error { @@ -1626,6 +1636,8 @@ func (fsm *storeFSM) Apply(l *raft.Log) interface{} { return fsm.applyCreateDatabaseCommand(&cmd) case internal.Command_DropDatabaseCommand: return fsm.applyDropDatabaseCommand(&cmd) + case internal.Command_RenameDatabaseCommand: + return fsm.applyRenameDatabaseCommand(&cmd) case internal.Command_CreateRetentionPolicyCommand: return fsm.applyCreateRetentionPolicyCommand(&cmd) case internal.Command_DropRetentionPolicyCommand: @@ -1751,6 +1763,20 @@ func (fsm *storeFSM) applyDropDatabaseCommand(cmd *internal.Command) interface{} return nil } +func (fsm *storeFSM) applyRenameDatabaseCommand(cmd *internal.Command) interface{} { + ext, _ := proto.GetExtension(cmd, internal.E_RenameDatabaseCommand_Command) + v := ext.(*internal.RenameDatabaseCommand) + + // Copy data and update. + other := fsm.data.Clone() + if err := other.RenameDatabase(v.GetOldName(), v.GetNewName()); err != nil { + return err + } + fsm.data = other + + return nil +} + func (fsm *storeFSM) applyCreateRetentionPolicyCommand(cmd *internal.Command) interface{} { ext, _ := proto.GetExtension(cmd, internal.E_CreateRetentionPolicyCommand_Command) v := ext.(*internal.CreateRetentionPolicyCommand) diff --git a/meta/store_test.go b/meta/store_test.go index ae8d9bf1f8..4affa2965f 100644 --- a/meta/store_test.go +++ b/meta/store_test.go @@ -244,6 +244,76 @@ func TestStore_DropDatabase_ErrDatabaseNotFound(t *testing.T) { } } +// Ensure the store can rename an existing database. +func TestStore_RenameDatabase(t *testing.T) { + t.Parallel() + s := MustOpenStore() + defer s.Close() + + // Create three databases. + for i := 0; i < 3; i++ { + if _, err := s.CreateDatabase(fmt.Sprintf("db%d", i)); err != nil { + t.Fatal(err) + } + } + + // Rename database db1, leaving db0 and db2 unchanged. + if err := s.RenameDatabase("db1", "db3"); err != nil { + t.Fatal(err) + } + + // Ensure the nodes are correct. + exp := &meta.DatabaseInfo{Name: "db0"} + if di, _ := s.Database("db0"); !reflect.DeepEqual(di, exp) { + t.Fatalf("unexpected database(0): \ngot: %#v\nexp: %#v", di, exp) + + } + if di, _ := s.Database("db1"); di != nil { + t.Fatalf("unexpected database(1): %#v", di) + } + + exp = &meta.DatabaseInfo{Name: "db2"} + if di, _ := s.Database("db2"); !reflect.DeepEqual(di, exp) { + t.Fatalf("unexpected database(2): \ngot: %#v\nexp: %#v", di, exp) + } + + exp = &meta.DatabaseInfo{Name: "db3"} + if di, _ := s.Database("db3"); !reflect.DeepEqual(di, exp) { + t.Fatalf("unexpected database(2): \ngot: %#v\nexp: %#v", di, exp) + } +} + +// Ensure the store returns an error when renaming a database that doesn't exist. +func TestStore_RenameDatabase_ErrDatabaseNotFound(t *testing.T) { + t.Parallel() + s := MustOpenStore() + defer s.Close() + + if err := s.RenameDatabase("no_such_database", "another_database"); err != meta.ErrDatabaseNotFound { + t.Fatalf("unexpected error: %s", err) + } +} + +// Ensure the store returns an error when renaming a database to a database that already exists. +func TestStore_RenameDatabase_ErrDatabaseExists(t *testing.T) { + t.Parallel() + s := MustOpenStore() + defer s.Close() + + // create two databases + if _, err := s.CreateDatabase("db00"); err != nil { + t.Fatal(err) + } + + if _, err := s.CreateDatabase("db01"); err != nil { + t.Fatal(err) + } + + if err := s.RenameDatabase("db00", "db01"); err != meta.ErrDatabaseExists { + t.Fatalf("unexpected error: %s", err) + } +} + // Ensure the store can create a retention policy on a database. func TestStore_CreateRetentionPolicy(t *testing.T) { t.Parallel() From d2afd881e67e63a31e9955f4d8ba1baed635ce51 Mon Sep 17 00:00:00 2001 From: linearb Date: Fri, 2 Oct 2015 10:01:20 -0400 Subject: [PATCH 2/7] add integration test for RENAME DATABASE --- cmd/influxd/run/server_test.go | 75 ++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index d24be1587d..72bc929fa1 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -266,6 +266,81 @@ func TestServer_Query_DropDatabaseIsolated(t *testing.T) { } } +func TestServer_Query_RenameDatabase(t *testing.T) { + t.Parallel() + s := OpenServer(NewConfig(), "") + defer s.Close() + + if err := s.CreateDatabaseAndRetentionPolicy("db0", newRetentionPolicyInfo("rp0", 1, 0)); err != nil { + t.Fatal(err) + } + if err := s.MetaStore.SetDefaultRetentionPolicy("db0", "rp0"); err != nil { + t.Fatal(err) + } + + writes := []string{ + fmt.Sprintf(`cpu,host=serverA,region=uswest val=23.2 %d`, mustParseTime(time.RFC3339Nano, "2000-01-01T00:00:00Z").UnixNano()), + } + + test := NewTest("db0", "rp0") + test.write = strings.Join(writes, "\n") + + test.addQueries([]*Query{ + &Query{ + name: "Query data from db0 database", + command: `SELECT * FROM cpu`, + exp: `{"results":[{"series":[{"name":"cpu","columns":["time","host","region","val"],"values":[["2000-01-01T00:00:00Z","serverA","uswest",23.2]]}]}]}`, + params: url.Values{"db": []string{"db0"}}, + }, + &Query{ + name: "Query data from db0 database with GROUP BY *", + command: `SELECT * FROM cpu GROUP BY *`, + exp: `{"results":[{"series":[{"name":"cpu","tags":{"host":"serverA","region":"uswest"},"columns":["time","val"],"values":[["2000-01-01T00:00:00Z",23.2]]}]}]}`, + params: url.Values{"db": []string{"db0"}}, + }, + &Query{ + name: "Rename database", + command: `RENAME DATABASE db0 to db1`, + exp: `{"results":[{}]}`, + }, + &Query{ + name: "Query data from db0 database and ensure it's gone", + command: `SELECT * FROM cpu`, + exp: `{"results":[{"error":"database not found: db0"}]}`, + params: url.Values{"db": []string{"db0"}}, + }, + &Query{ + name: "Query data from now renamed database db1 and ensure that's there", + command: `SELECT * FROM cpu`, + exp: `{"results":[{"series":[{"name":"cpu","columns":["time","host","region","val"],"values":[["2000-01-01T00:00:00Z","serverA","uswest",23.2]]}]}]}`, + params: url.Values{"db": []string{"db1"}}, + }, + &Query{ + name: "Query data from now renamed database db1 and ensure it's still there with GROUP BY *", + command: `SELECT * FROM cpu GROUP BY *`, + exp: `{"results":[{"series":[{"name":"cpu","tags":{"host":"serverA","region":"uswest"},"columns":["time","val"],"values":[["2000-01-01T00:00:00Z",23.2]]}]}]}`, + params: url.Values{"db": []string{"db1"}}, + }, + }...) + + for i, query := range test.queries { + if i == 0 { + if err := test.init(s); err != nil { + t.Fatalf("test init failed: %s", err) + } + } + if query.skip { + t.Logf("SKIP:: %s", query.name) + continue + } + if err := query.Execute(s); err != nil { + t.Error(query.Error(err)) + } else if !query.success() { + t.Error(query.failureMessage()) + } + } +} + func TestServer_Query_DropAndRecreateSeries(t *testing.T) { t.Parallel() s := OpenServer(NewConfig(), "") From 668b5b9bfb88ef475d68cd3dce3b21ef96fa4de7 Mon Sep 17 00:00:00 2001 From: linearb Date: Fri, 2 Oct 2015 22:23:42 -0400 Subject: [PATCH 3/7] change syntax to ALTER DATABASE ... RENAME TO ... --- cmd/influxd/run/server_test.go | 12 +++++----- influxql/ast.go | 17 +++++++------- influxql/parser.go | 41 ++++++++++++++++----------------- influxql/parser_test.go | 25 +++++++++++++++++--- meta/statement_executor.go | 6 ++--- meta/statement_executor_test.go | 6 ++--- 6 files changed, 63 insertions(+), 44 deletions(-) diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index 72bc929fa1..2a25fffe7e 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -68,7 +68,7 @@ func TestServer_DatabaseCommands(t *testing.T) { }, &Query{ name: "rename database should succeed", - command: `RENAME DATABASE db1 TO db2`, + command: `ALTER DATABASE db1 RENAME TO db2`, exp: `{"results":[{}]}`, }, &Query{ @@ -78,17 +78,17 @@ func TestServer_DatabaseCommands(t *testing.T) { }, &Query{ name: "rename non-existent database should fail", - command: `RENAME DATABASE db4 TO db5`, + command: `ALTER DATABASE db4 RENAME TO db5`, exp: `{"results":[{"error":"database not found"}]}`, }, &Query{ name: "rename database to illegal name should fail", - command: `RENAME DATABASE db2 TO 0xdb0`, - exp: `{"error":"error parsing query: found 0, expected identifier at line 1, char 24"}`, + command: `ALTER DATABASE db2 RENAME TO 0xdb0`, + exp: `{"error":"error parsing query: found 0, expected identifier at line 1, char 30"}`, }, &Query{ name: "rename database to already existing datbase should fail", - command: `RENAME DATABASE db2 TO db0`, + command: `ALTER DATABASE db2 RENAME TO db0`, exp: `{"results":[{"error":"database already exists"}]}`, }, &Query{ @@ -300,7 +300,7 @@ func TestServer_Query_RenameDatabase(t *testing.T) { }, &Query{ name: "Rename database", - command: `RENAME DATABASE db0 to db1`, + command: `ALTER DATABASE db0 RENAME TO db1`, exp: `{"results":[{}]}`, }, &Query{ diff --git a/influxql/ast.go b/influxql/ast.go index 75b439c7a1..9c3d09f6c6 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -80,6 +80,7 @@ type Node interface { func (*Query) node() {} func (Statements) node() {} +func (*AlterDatabaseRenameStatement) node() {} func (*AlterRetentionPolicyStatement) node() {} func (*CreateContinuousQueryStatement) node() {} func (*CreateDatabaseStatement) node() {} @@ -96,7 +97,6 @@ func (*DropServerStatement) node() {} func (*DropUserStatement) node() {} func (*GrantStatement) node() {} func (*GrantAdminStatement) node() {} -func (*RenameDatabaseStatement) node() {} func (*RevokeStatement) node() {} func (*RevokeAdminStatement) node() {} func (*SelectStatement) node() {} @@ -189,6 +189,7 @@ type ExecutionPrivilege struct { // ExecutionPrivileges is a list of privileges required to execute a statement. type ExecutionPrivileges []ExecutionPrivilege +func (*AlterDatabaseRenameStatement) stmt() {} func (*AlterRetentionPolicyStatement) stmt() {} func (*CreateContinuousQueryStatement) stmt() {} func (*CreateDatabaseStatement) stmt() {} @@ -204,7 +205,6 @@ func (*DropServerStatement) stmt() {} func (*DropUserStatement) stmt() {} func (*GrantStatement) stmt() {} func (*GrantAdminStatement) stmt() {} -func (*RenameDatabaseStatement) stmt() {} func (*ShowContinuousQueriesStatement) stmt() {} func (*ShowGrantsForUserStatement) stmt() {} func (*ShowServersStatement) stmt() {} @@ -504,8 +504,8 @@ func (s *GrantAdminStatement) RequiredPrivileges() ExecutionPrivileges { return ExecutionPrivileges{{Admin: true, Name: "", Privilege: AllPrivileges}} } -// RenameDatabaseStatement represents a command for renaming a database. -type RenameDatabaseStatement struct { +// AlterDatabaseRenameStatement represents a command for renaming a database. +type AlterDatabaseRenameStatement struct { // Current name of the database OldName string // New name of the database @@ -513,17 +513,18 @@ type RenameDatabaseStatement struct { } // String returns a string representation of the rename database statement. -func (s *RenameDatabaseStatement) String() string { +func (s *AlterDatabaseRenameStatement) String() string { var buf bytes.Buffer - _, _ = buf.WriteString("RENAME DATABASE ") + _, _ = buf.WriteString("ALTER DATABASE ") _, _ = buf.WriteString(s.OldName) + _, _ = buf.WriteString(" RENAME ") _, _ = buf.WriteString(" TO ") _, _ = buf.WriteString(s.NewName) return buf.String() } -// RequiredPrivileges returns the privilege required to execute a RenameDatabaseStatement. -func (s *RenameDatabaseStatement) RequiredPrivileges() ExecutionPrivileges { +// RequiredPrivileges returns the privilege required to execute an AlterDatabaseRenameStatement. +func (s *AlterDatabaseRenameStatement) RequiredPrivileges() ExecutionPrivileges { return ExecutionPrivileges{{Admin: true, Name: "", Privilege: AllPrivileges}} } diff --git a/influxql/parser.go b/influxql/parser.go index 482582d041..2785bc7e41 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -90,8 +90,6 @@ func (p *Parser) ParseStatement() (Statement, error) { return p.parseDropStatement() case GRANT: return p.parseGrantStatement() - case RENAME: - return p.parseRenameStatement() case REVOKE: return p.parseRevokeStatement() case ALTER: @@ -99,7 +97,7 @@ func (p *Parser) ParseStatement() (Statement, error) { case SET: return p.parseSetPasswordUserStatement() default: - return nil, newParseError(tokstr(tok, lit), []string{"SELECT", "DELETE", "SHOW", "CREATE", "DROP", "GRANT", "RENAME", "REVOKE", "ALTER", "SET"}, pos) + return nil, newParseError(tokstr(tok, lit), []string{"SELECT", "DELETE", "SHOW", "CREATE", "DROP", "GRANT", "REVOKE", "ALTER", "SET"}, pos) } } @@ -221,25 +219,18 @@ func (p *Parser) parseDropStatement() (Statement, error) { // This function assumes the ALTER token has already been consumed. func (p *Parser) parseAlterStatement() (Statement, error) { tok, pos, lit := p.scanIgnoreWhitespace() - if tok == RETENTION { + + switch tok { + case RETENTION: if tok, pos, lit = p.scanIgnoreWhitespace(); tok != POLICY { return nil, newParseError(tokstr(tok, lit), []string{"POLICY"}, pos) } return p.parseAlterRetentionPolicyStatement() + case DATABASE: + return p.parseAlterDatabaseRenameStatement() } - return nil, newParseError(tokstr(tok, lit), []string{"RETENTION"}, pos) -} - -// parseRenameStatement parses a string and returns a rename statement. -// This function assumes the RENAME token has already been consumed. -func (p *Parser) parseRenameStatement() (Statement, error) { - tok, pos, lit := p.scanIgnoreWhitespace() - if tok == DATABASE { - return p.parseRenameDatabaseStatement() - } - - return nil, newParseError(tokstr(tok, lit), []string{"DATABASE"}, pos) + return nil, newParseError(tokstr(tok, lit), []string{"RETENTION", "DATABASE"}, pos) } // parseDropStatement parses a string and returns a drop statement. @@ -1367,10 +1358,10 @@ func (p *Parser) parseDropDatabaseStatement() (*DropDatabaseStatement, error) { return stmt, nil } -// parseRenameDatabaseStatement parses a string and returns a RenameDatabaseStatement. -// This function assumes the "RENAME DATABASE" tokens have already been consumed. -func (p *Parser) parseRenameDatabaseStatement() (*RenameDatabaseStatement, error) { - stmt := &RenameDatabaseStatement{} +// parseAlterDatabaseRenameStatement parses a string and returns an AlterDatabaseRenameStatement. +// This function assumes the "ALTER DATABASE" tokens have already been consumed. +func (p *Parser) parseAlterDatabaseRenameStatement() (*AlterDatabaseRenameStatement, error) { + stmt := &AlterDatabaseRenameStatement{} // Parse the name of the database to be renamed. lit, err := p.parseIdent() @@ -1379,9 +1370,17 @@ func (p *Parser) parseRenameDatabaseStatement() (*RenameDatabaseStatement, error } stmt.OldName = lit - // Parse TO clause. + // Parse RENAME clause. tok, pos, lit := p.scanIgnoreWhitespace() + // Check for required RENAME token. + if tok != RENAME { + return nil, newParseError(tokstr(tok, lit), []string{"RENAME"}, pos) + } + + // Parse TO clause. + tok, pos, lit = p.scanIgnoreWhitespace() + // Check for required TO token. if tok != TO { return nil, newParseError(tokstr(tok, lit), []string{"TO"}, pos) diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 71ea830285..5973e71184 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -1412,12 +1412,19 @@ func TestParser_ParseStatement(t *testing.T) { s: `ALTER RETENTION POLICY policy1 ON testdb REPLICATION 4`, stmt: newAlterRetentionPolicyStatement("policy1", "testdb", -1, 4, false), }, + // ALTER default retention policy unquoted { s: `ALTER RETENTION POLICY default ON testdb REPLICATION 4`, stmt: newAlterRetentionPolicyStatement("default", "testdb", -1, 4, false), }, + // ALTER DATABASE RENAME + { + s: `ALTER DATABASE db0 RENAME TO db1`, + stmt: newAlterDatabaseRenameStatement("db0", "db1"), + }, + // SHOW STATS { s: `SHOW STATS`, @@ -1451,10 +1458,10 @@ func TestParser_ParseStatement(t *testing.T) { }, // Errors - {s: ``, err: `found EOF, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, RENAME, REVOKE, ALTER, SET at line 1, char 1`}, + {s: ``, err: `found EOF, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, REVOKE, ALTER, SET at line 1, char 1`}, {s: `SELECT`, err: `found EOF, expected identifier, string, number, bool at line 1, char 8`}, {s: `SELECT time FROM myseries`, err: `at least 1 non-time field must be queried`}, - {s: `blah blah`, err: `found blah, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, RENAME, REVOKE, ALTER, SET at line 1, char 1`}, + {s: `blah blah`, err: `found blah, expected SELECT, DELETE, SHOW, CREATE, DROP, GRANT, REVOKE, ALTER, SET at line 1, char 1`}, {s: `SELECT field1 X`, err: `found X, expected FROM at line 1, char 15`}, {s: `SELECT field1 FROM "series" WHERE X +;`, err: `found ;, expected identifier, string, number, bool at line 1, char 38`}, {s: `SELECT field1 FROM myseries GROUP`, err: `found EOF, expected BY at line 1, char 35`}, @@ -1639,11 +1646,15 @@ func TestParser_ParseStatement(t *testing.T) { {s: `CREATE RETENTION POLICY policy1 ON testdb DURATION 1h REPLICATION 0`, err: `invalid value 0: must be 1 <= n <= 2147483647 at line 1, char 67`}, {s: `CREATE RETENTION POLICY policy1 ON testdb DURATION 1h REPLICATION bad`, err: `found bad, expected number at line 1, char 67`}, {s: `CREATE RETENTION POLICY policy1 ON testdb DURATION 1h REPLICATION 1 foo`, err: `found foo, expected DEFAULT at line 1, char 69`}, - {s: `ALTER`, err: `found EOF, expected RETENTION at line 1, char 7`}, + {s: `ALTER`, err: `found EOF, expected RETENTION, DATABASE at line 1, char 7`}, {s: `ALTER RETENTION`, err: `found EOF, expected POLICY at line 1, char 17`}, {s: `ALTER RETENTION POLICY`, err: `found EOF, expected identifier at line 1, char 24`}, {s: `ALTER RETENTION POLICY policy1`, err: `found EOF, expected ON at line 1, char 32`}, {s: `ALTER RETENTION POLICY policy1 ON`, err: `found EOF, expected identifier at line 1, char 35`}, {s: `ALTER RETENTION POLICY policy1 ON testdb`, err: `found EOF, expected DURATION, RETENTION, DEFAULT at line 1, char 42`}, + {s: `ALTER DATABASE`, err: `found EOF, expected identifier at line 1, char 16`}, + {s: `ALTER DATABASE db0`, err: `found EOF, expected RENAME at line 1, char 20`}, + {s: `ALTER DATABASE db0 RENAME`, err: `found EOF, expected TO at line 1, char 27`}, + {s: `ALTER DATABASE db0 RENAME TO`, err: `found EOF, expected identifier at line 1, char 30`}, {s: `SET`, err: `found EOF, expected PASSWORD at line 1, char 5`}, {s: `SET PASSWORD`, err: `found EOF, expected FOR at line 1, char 14`}, {s: `SET PASSWORD something`, err: `found something, expected FOR at line 1, char 14`}, @@ -2077,6 +2088,14 @@ func newAlterRetentionPolicyStatement(name string, DB string, d time.Duration, r return stmt } +// newAlterDatabaseRenameStatement creates an initialized AlterDatabaseRenameStatement. +func newAlterDatabaseRenameStatement(oldName, newName string) *influxql.AlterDatabaseRenameStatement { + return &influxql.AlterDatabaseRenameStatement{ + OldName: oldName, + NewName: newName, + } +} + // mustMarshalJSON encodes a value to JSON. func mustMarshalJSON(v interface{}) []byte { b, err := json.Marshal(v) diff --git a/meta/statement_executor.go b/meta/statement_executor.go index 7899aa0c62..6cd7afc7dd 100644 --- a/meta/statement_executor.go +++ b/meta/statement_executor.go @@ -70,8 +70,8 @@ func (e *StatementExecutor) ExecuteStatement(stmt influxql.Statement) *influxql. return e.executeGrantStatement(stmt) case *influxql.GrantAdminStatement: return e.executeGrantAdminStatement(stmt) - case *influxql.RenameDatabaseStatement: - return e.executeRenameDatabaseStatement(stmt) + case *influxql.AlterDatabaseRenameStatement: + return e.executeAlterDatabaseRenameStatement(stmt) case *influxql.RevokeStatement: return e.executeRevokeStatement(stmt) case *influxql.RevokeAdminStatement: @@ -215,7 +215,7 @@ func (e *StatementExecutor) executeGrantAdminStatement(stmt *influxql.GrantAdmin return &influxql.Result{Err: e.Store.SetAdminPrivilege(stmt.User, true)} } -func (e *StatementExecutor) executeRenameDatabaseStatement(q *influxql.RenameDatabaseStatement) *influxql.Result { +func (e *StatementExecutor) executeAlterDatabaseRenameStatement(q *influxql.AlterDatabaseRenameStatement) *influxql.Result { return &influxql.Result{Err: e.Store.RenameDatabase(q.OldName, q.NewName)} } diff --git a/meta/statement_executor_test.go b/meta/statement_executor_test.go index 713cd1af5d..eaaa5cf8b6 100644 --- a/meta/statement_executor_test.go +++ b/meta/statement_executor_test.go @@ -46,8 +46,8 @@ func TestStatementExecutor_ExecuteStatement_DropDatabase(t *testing.T) { } } -// Ensure a RENAME DATABASE statement can be executed. -func TestStatementExecutor_ExecuteStatement_RenameDatabase(t *testing.T) { +// Ensure an ALTER DATABASE ... RENAME TO ... statement can be executed. +func TestStatementExecutor_ExecuteStatement_AlterDatabaseRename(t *testing.T) { e := NewStatementExecutor() e.Store.RenameDatabaseFn = func(oldName, newName string) error { if oldName != "old_foo" { @@ -59,7 +59,7 @@ func TestStatementExecutor_ExecuteStatement_RenameDatabase(t *testing.T) { return nil } - if res := e.ExecuteStatement(influxql.MustParseStatement(`RENAME DATABASE old_foo to new_foo`)); res.Err != nil { + if res := e.ExecuteStatement(influxql.MustParseStatement(`ALTER DATABASE old_foo RENAME TO new_foo`)); res.Err != nil { t.Fatal(res.Err) } else if res.Series != nil { t.Fatalf("unexpected rows: %#v", res.Series) From 1383d964aaccf932339fb4c69852865eb2238584 Mon Sep 17 00:00:00 2001 From: linearb Date: Tue, 6 Oct 2015 22:01:53 -0400 Subject: [PATCH 4/7] update user privileges on database rename --- meta/data.go | 14 +++++++++++++- meta/data_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/meta/data.go b/meta/data.go index 647a5fbdbc..994526f4a3 100644 --- a/meta/data.go +++ b/meta/data.go @@ -195,13 +195,25 @@ func (data *Data) RenameDatabase(oldName, newName string) error { for i := range data.Databases { if data.Databases[i].Name == oldName { data.Databases[i].Name = newName - //TODO CQs + data.switchDatabaseUserPrivileges(oldName, newName) + // TODO should rename the databases used in continuous queries return nil } } return ErrDatabaseNotFound } +// switchDatabaseUserPrivileges changes the database associated with user privileges +func (data *Data) switchDatabaseUserPrivileges(oldDatabase, newDatabase string) error { + for i := range data.Users { + if p, ok := data.Users[i].Privileges[oldDatabase]; ok { + data.Users[i].Privileges[newDatabase] = p + delete(data.Users[i].Privileges, oldDatabase) + } + } + return nil +} + // RetentionPolicy returns a retention policy for a database by name. func (data *Data) RetentionPolicy(database, name string) (*RetentionPolicyInfo, error) { di := data.Database(database) diff --git a/meta/data_test.go b/meta/data_test.go index 482d22c321..af3e5fb669 100644 --- a/meta/data_test.go +++ b/meta/data_test.go @@ -151,6 +151,35 @@ func TestData_RenameDatabase(t *testing.T) { } } +// Ensure that user privileges are updated correctly when database is renamed. +func TestData_RenameDatabaseUpdatesPrivileges(t *testing.T) { + var data meta.Data + for i := 0; i < 2; i++ { + if err := data.CreateDatabase(fmt.Sprintf("db%d", i)); err != nil { + t.Fatal(err) + } + } + + data.Users = []meta.UserInfo{{ + Name: "susy", + Hash: "ABC123", + Admin: true, + Privileges: map[string]influxql.Privilege{ + "db1": influxql.AllPrivileges, "db0": influxql.ReadPrivilege}}} + + if err := data.RenameDatabase("db1", "db2"); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(data.Users, + []meta.UserInfo{{ + Name: "susy", + Hash: "ABC123", + Admin: true, + Privileges: map[string]influxql.Privilege{ + "db2": influxql.AllPrivileges, "db0": influxql.ReadPrivilege}}}) { + t.Fatalf("unexpected user privileges: %#v", data.Users) + } +} + // Ensure that renaming a database without both old and new names returns an error. func TestData_RenameDatabase_ErrNameRequired(t *testing.T) { var data meta.Data From 218de0acbb8cebd10c12bda03688d9fc3505a7b7 Mon Sep 17 00:00:00 2001 From: linearb Date: Tue, 6 Oct 2015 22:22:44 -0400 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e8879c44..d6721c19fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - [#4310](https://github.com/influxdb/influxdb/pull/4310): Support dropping non-Raft nodes. Work mostly by @corylanou - [#4348](https://github.com/influxdb/influxdb/pull/4348): Public ApplyTemplate function for graphite parser. - [#4178](https://github.com/influxdb/influxdb/pull/4178): Support fields in graphite parser. Thanks @roobert! +- [#4291](https://github.com/influxdb/influxdb/pull/4291): Added ALTER DATABASE RENAME ### Bugfixes - [#4166](https://github.com/influxdb/influxdb/pull/4166): Fix parser error on invalid SHOW From 37964a032f2f0d149465df538a5ade5644b69f11 Mon Sep 17 00:00:00 2001 From: linearb Date: Wed, 7 Oct 2015 15:08:49 -0400 Subject: [PATCH 6/7] improvements from code review --- influxql/parser.go | 19 +++---------------- influxql/parser_test.go | 1 - meta/internal/meta.proto | 2 +- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/influxql/parser.go b/influxql/parser.go index 2785bc7e41..23a3a7eb5a 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -233,8 +233,6 @@ func (p *Parser) parseAlterStatement() (Statement, error) { return nil, newParseError(tokstr(tok, lit), []string{"RETENTION", "DATABASE"}, pos) } -// parseDropStatement parses a string and returns a drop statement. - // parseSetPasswordUserStatement parses a string and returns a set statement. // This function assumes the SET token has already been consumed. func (p *Parser) parseSetPasswordUserStatement() (*SetPasswordUserStatement, error) { @@ -1370,20 +1368,9 @@ func (p *Parser) parseAlterDatabaseRenameStatement() (*AlterDatabaseRenameStatem } stmt.OldName = lit - // Parse RENAME clause. - tok, pos, lit := p.scanIgnoreWhitespace() - - // Check for required RENAME token. - if tok != RENAME { - return nil, newParseError(tokstr(tok, lit), []string{"RENAME"}, pos) - } - - // Parse TO clause. - tok, pos, lit = p.scanIgnoreWhitespace() - - // Check for required TO token. - if tok != TO { - return nil, newParseError(tokstr(tok, lit), []string{"TO"}, pos) + // Parse required RENAME TO tokens. + if err := p.parseTokens([]Token{RENAME, TO}); err != nil { + return nil, err } // Parse the new name of the database. diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 5973e71184..a70a79ed55 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -1412,7 +1412,6 @@ func TestParser_ParseStatement(t *testing.T) { s: `ALTER RETENTION POLICY policy1 ON testdb REPLICATION 4`, stmt: newAlterRetentionPolicyStatement("policy1", "testdb", -1, 4, false), }, - // ALTER default retention policy unquoted { s: `ALTER RETENTION POLICY default ON testdb REPLICATION 4`, diff --git a/meta/internal/meta.proto b/meta/internal/meta.proto index 178d5057d1..d3a9994bc8 100644 --- a/meta/internal/meta.proto +++ b/meta/internal/meta.proto @@ -105,7 +105,7 @@ message Command { SetDataCommand = 17; SetAdminPrivilegeCommand = 18; UpdateNodeCommand = 19; - RenameDatabaseCommand = 20; + RenameDatabaseCommand = 20; } required Type type = 1; From 60d298936eab636d984d3aff64732aaadc4a40b2 Mon Sep 17 00:00:00 2001 From: linearb Date: Fri, 9 Oct 2015 13:36:01 -0400 Subject: [PATCH 7/7] return error when database rename conflicts with continuous queries --- cmd/influxd/run/server_test.go | 17 ++++++++++++++++- meta/data.go | 30 +++++++++++++++++++++++++++++- meta/data_test.go | 32 ++++++++++++++++++++++++++++++++ meta/errors.go | 3 +++ 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index 2a25fffe7e..57f7ed8493 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -299,7 +299,22 @@ func TestServer_Query_RenameDatabase(t *testing.T) { params: url.Values{"db": []string{"db0"}}, }, &Query{ - name: "Rename database", + name: "Create continuous query using db0", + command: `CREATE CONTINUOUS QUERY "cq1" ON db0 BEGIN SELECT count(value) INTO "rp1".:MEASUREMENT FROM cpu GROUP BY time(5s) END`, + exp: `{"results":[{}]}`, + }, + &Query{ + name: "Rename database should fail because of conflicting CQ", + command: `ALTER DATABASE db0 RENAME TO db1`, + exp: `{"results":[{"error":"database rename conflict with existing continuous query"}]}`, + }, + &Query{ + name: "Drop conflicting CQ", + command: `DROP CONTINUOUS QUERY "cq1" on db0`, + exp: `{"results":[{}]}`, + }, + &Query{ + name: "Rename database should succeed now", command: `ALTER DATABASE db0 RENAME TO db1`, exp: `{"results":[{}]}`, }, diff --git a/meta/data.go b/meta/data.go index 994526f4a3..8d6ebf6aec 100644 --- a/meta/data.go +++ b/meta/data.go @@ -1,7 +1,9 @@ package meta import ( + "fmt" "sort" + "strings" "time" "github.com/gogo/protobuf/proto" @@ -191,18 +193,44 @@ func (data *Data) RenameDatabase(oldName, newName string) error { if data.Database(oldName) == nil { return ErrDatabaseNotFound } + // TODO should rename database in continuous queries also + // for now, just return an error if there is a possible conflict + if data.isDatabaseNameUsedInCQ(oldName) { + return ErrDatabaseRenameCQConflict + } // find database named oldName and rename it to newName for i := range data.Databases { if data.Databases[i].Name == oldName { data.Databases[i].Name = newName data.switchDatabaseUserPrivileges(oldName, newName) - // TODO should rename the databases used in continuous queries return nil } } return ErrDatabaseNotFound } +// isDatabaseNameUsedInCQ returns true if a database name is used in any continuous query +func (data *Data) isDatabaseNameUsedInCQ(dbName string) bool { + CQOnDb := fmt.Sprintf(" ON %s ", dbName) + CQIntoDb := fmt.Sprintf(" INTO \"%s\".", dbName) + CQFromDb := fmt.Sprintf(" FROM \"%s\".", dbName) + for i := range data.Databases { + for j := range data.Databases[i].ContinuousQueries { + query := data.Databases[i].ContinuousQueries[j].Query + if strings.Contains(query, CQOnDb) { + return true + } + if strings.Contains(query, CQIntoDb) { + return true + } + if strings.Contains(query, CQFromDb) { + return true + } + } + } + return false +} + // switchDatabaseUserPrivileges changes the database associated with user privileges func (data *Data) switchDatabaseUserPrivileges(oldDatabase, newDatabase string) error { for i := range data.Users { diff --git a/meta/data_test.go b/meta/data_test.go index af3e5fb669..f1088e9c91 100644 --- a/meta/data_test.go +++ b/meta/data_test.go @@ -194,6 +194,38 @@ func TestData_RenameDatabase_ErrNameRequired(t *testing.T) { } } +// Ensure that renaming a database returns an error if there is a possibly conflicting CQ +func TestData_RenameDatabase_ErrDatabaseCQConflict(t *testing.T) { + var data meta.Data + if err := data.CreateDatabase("db0"); err != nil { + t.Fatal(err) + } else if err := data.CreateDatabase("db1"); err != nil { + t.Fatal(err) + } else if err := data.CreateContinuousQuery("db0", "cq0", `CREATE CONTINUOUS QUERY cq0 ON db0 BEGIN SELECT count() INTO "foo"."default"."bar" FROM "foo"."foobar" END`); err != nil { + t.Fatal(err) + } else if err := data.CreateContinuousQuery("db1", "cq1", `CREATE CONTINUOUS QUERY cq1 ON db1 BEGIN SELECT count() INTO "db1"."default"."bar" FROM "db0"."foobar" END`); err != nil { + t.Fatal(err) + } else if err := data.CreateContinuousQuery("db1", "cq2", `CREATE CONTINUOUS QUERY cq2 ON db1 BEGIN SELECT count() INTO "db0"."default"."bar" FROM "db1"."foobar" END`); err != nil { + t.Fatal(err) + } else if err := data.CreateContinuousQuery("db1", "noconflict", `CREATE CONTINUOUS QUERY noconflict ON db1 BEGIN SELECT count() INTO "db1"."default"."bar" FROM "db1"."foobar" END`); err != nil { + t.Fatal(err) + } else if err := data.RenameDatabase("db0", "db2"); err == nil { + t.Fatalf("unexpected rename database success despite cq conflict") + } else if err := data.DropContinuousQuery("db0", "cq0"); err != nil { + t.Fatal(err) + } else if err := data.RenameDatabase("db0", "db2"); err == nil { + t.Fatalf("unexpected rename database success despite cq conflict") + } else if err := data.DropContinuousQuery("db1", "cq1"); err != nil { + t.Fatal(err) + } else if err := data.RenameDatabase("db0", "db2"); err == nil { + t.Fatalf("unexpected rename database success despite cq conflict") + } else if err := data.DropContinuousQuery("db1", "cq2"); err != nil { + t.Fatal(err) + } else if err := data.RenameDatabase("db0", "db2"); err != nil { + t.Fatal(err) + } +} + // Ensure a retention policy can be created. func TestData_CreateRetentionPolicy(t *testing.T) { data := meta.Data{Nodes: []meta.NodeInfo{{ID: 1}, {ID: 2}}} diff --git a/meta/errors.go b/meta/errors.go index 213b102f4e..26151c92d9 100644 --- a/meta/errors.go +++ b/meta/errors.go @@ -47,6 +47,9 @@ var ( // ErrDatabaseNameRequired is returned when creating a database without a name. ErrDatabaseNameRequired = newError("database name required") + + // ErrDatabaseRenameCQConflict is returned when attempting to rename a database in use by a CQ. + ErrDatabaseRenameCQConflict = newError("database rename conflict with existing continuous query") ) var (