add method MetaAPI::rename_table()

[drmingdrmer]

async fn rename_table(&self, req : RenameTableReq) -> Result<RenameTableReply>

struct RenameTableReq {
    db_id: u64,
    table_name: String,
    new_table_name: String
}

This method will remove old table record and add a new one in a sled-tree-transaction.

The impl would be similar to CreateTable:
https://github.com/datafuselabs/databend/blob/77fe82ff05ebd8c8c689101bb64a9f53cd50133f/common/meta/raft-store/src/state_machine/sm.rs#L482-L495

BTW, There may be consistency concern: databend-query currently just loads table by (database-name, table-name).

---

Originally posted by @BohuTANG in #3557 (comment)

[kevinw66]

Why not use alter_table for more common usage ?
According to the document https://dev.mysql.com/doc/refman/8.0/en/rename-table.html
We can find rename table_1 to table_2 and alter table_1 rename table_2 has the same behavior.

[drmingdrmer]

This is an API to operate the underlying kv store(the meta data store).
It's hard to capsulate many variants into one single method(as sql does).

Here this method just rename a table, and does nothing more else.
Intuitively ,alter_table should include changing table schema etc.

[junnplus]

/assignme

I take this issue to learn meta api.

[drmingdrmer]

/assignme

I take this issue to learn meta API.

@ariesdevil has an ongoing working branch that may have conflicts with this issue.
You may like to have a discussion about the work with him. 🤔

• MetaApi supports multi tenant #3759

[ariesdevil]

@junnplus Here you go~

[junnplus]

I might need help. : (

I try to add rename method to meta api:

1. rename needs to be done on a transaction: remove old table record and add a new one.
   That is, we need to return a ErrorCode::UnknownTable / TableAlreadyExists instead of AppliedState in the sled transaction. But before https://github.com/datafuselabs/databend/pull/3837/files I had a hard time getting the errors in the transaction, Thank @ariesdevil
2. But then ErrorCode::UnknownTable will be wrapped into ErrorCode::TransactionAbort, it doesn't seem to be a problem, I can get the original error with cause
3. The most troublesome is in the grpc handler: I got a error like Err(UnknownError("Code: 2102, displayText = Transaction abort, cause: Code: 1025, displayText = Unknown table: 'tb2'.\nCode: 1025, displayText = Unknown table: 'tb2'..")).
   Yes, it is a MetaError, maybe I can get the original error, but that's very inelegant.

I'm not sure if I'm missing something I don't know. : (

@drmingdrmer @ariesdevil

[drmingdrmer]

3. The most troublesome is in the grpc handler: I got a error like Err(UnknownError("Code: 2102, displayText = Transaction abort, cause: Code: 1025, displayText = Unknown table: 'tb2'.\nCode: 1025, displayText = Unknown table: 'tb2'..")).
Yes, it is a MetaError, maybe I can get the original error, but that's very inelegant.

In general, ErrorCode is not a good choice in such a case.

And a failure to rename a table should not emit a MetaError. MetaError is meant for server error, not an application error.

AppliedState is meant to express application-data changes in the state machine.
AppliedState contains the two states before and after applying a log entry.
Although, for now, it can not express state change about renaming table. Maybe we need to add another enum entry, such as TableBla(Change<(TableName, TableId)>):

• Success: before: (name_1, id_1), after: (name_2, id_1).
• Unknown table: before:None, after: None.
• Target name already exists: before: (name_2, id_2), after: (name_2, id_2).

It might be working, but not quite sure if it is a good idea

[junnplus]

@drmingdrmer It seems that using AppliedState as the return cannot rollback the transaction. If the drop table succeeds and the create table fails, the transaction cannot be rolled back.

[ariesdevil]

@drmingdrmer It seems that using AppliedState as the return cannot rollback the transaction. If the drop table succeeds and the create table fails, the transaction cannot be rolled back.

AppliedState just part of Result<AppliedState, ErrorCode>，so there is an error that transaction can handle and rollback

[drmingdrmer]

What if we remove ErrorCode, and make UnknownTable and TableExists sub errors of MetaError.
IMHO, that every crate has its own hierarchical error types would be better than using non-structured errors

[ariesdevil]

What if we remove ErrorCode, and make UnknownTable and TableExists sub errors of MetaError. IMHO, that every crate has its own hierarchical error types would be better than using non-structured errors

ref: #4000

[drmingdrmer]

AppliedState will be able to express an application error such as UnknownDatabase after the following pr is merged:

• [metasrv] fix: app-error should not fail state machine txn #4099

It looks good enough to support the expected modification in this issue? :D

[junnplus]

Sorry for the late reply. :(

#4099 very useful for me, big thanks! I continue to work on this issue at #4288 .