Unify all usage of table full name to a canonical struct.

[MichaelScofield]

What type of enhancement is this?

Refactor, Tech debt reduction

What does the enhancement do?

Currently we are using different methods to refer to a table's full name. For example, TableName, (catalog_name, schema_name, table_name) or TableReference. It would be better if we could eliminate the difference, unify them to a canonical struct used everywhere. I prefer TableReference.

Implementation challenges

Replace all references to a table's full name with TableReference.

[waynexia]

Agree. But TableReference might be confused with TableRef. How about TableDescriptor (out of fd file descriptor).

Actually I prefer TableIdentifier but it's occupied by table id 🫠

pub struct TableIdent {
    /// Unique id of this table.
    pub table_id: TableId,
    /// Version of the table, bumped when metadata (such as schema) of the table
    /// being changed.
    pub version: TableVersion,
}

[MichaelScofield]

How about TableFullName, simple, straightforward, can't be more understandable.

[iwinux]

What about TablePath

[waynexia]

This topic is so interesting and I open a vote discussion for it: Combine catalog name, schema name and table name together!

[waynexia]

From the time I comment, TableFullName is the most preferred naming 🥳

@iwinux are you willing to make this change?

[iwinux]

Yes I can take this task.

[waynexia]

Yes I can take this task.

Thanks :heart:!

[iwinux]

TODOs:

• [ ] rename table::engine::TableReference (introduced in #491) to TableFullName
• [ ] replace all occurrences of tuple (catalog_name, schema_name, table_name) with TableFullName (could be found by rg '\(catalog\S+, schema\S+,')
• [ ] deal with meta_client::rpc::TableName

Not sure how to handle meta_client::rpc::TableName yet - table and meta-client should not import each other, right?

[MichaelScofield]

Right, but I think you should be able to dependent on table in meta-client. table does not dependent on meta-client

2022年11月23日 17:00，Limbo Peng @.***> 写道：

TODOs:

rename table::engine::TableReference (introduced in #491 https://github.com/GreptimeTeam/greptimedb/pull/491) to TableFullName replace all occurrences of tuple (catalog_name, schema_name, table_name) with TableFullName (could be found by rg '(catalog\S+, schema\S+,') deal with meta_client::rpc::TableName Not sure how to handle meta_client::rpc::TableName yet - table and meta-client should not import each other, right?

— Reply to this email directly, view it on GitHub https://github.com/GreptimeTeam/greptimedb/issues/559#issuecomment-1324732433, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHR2D67PJGBRQUGX6ZBMGTWJXML7ANCNFSM6AAAAAASDH4DBI. You are receiving this because you authored the thread.

[MichaelScofield]

ping @iwinux , any updates?

[iwinux]

Sorry but no progress has been made. Please reassign this issue as you see fit.

Apologize for the delay of updates :(

[MichaelScofield]

It's ok, I'll handle it.

[qstommyshu]

Hi, Just wondering if this issue has been handled? If not, I can probably take a look at it(I'm currently looking for good first issue to get started with contributing to greptime db).

[MichaelScofield]

@qstommyshu feel free to submit PRs to this issue. You can do it progressively, modify part of the codes at a time.

[qstommyshu]

Thanks for replying!

I was looking at the previous discussion and reading the code related to this issue. From the todos given by @iwinux , I think the first two checkpoints can be done by renaming, but I don't think I understand "Not sure how to handle meta_client::rpc::TableName yet - table and meta-client should not import each other, right?"(Question: why they should not import each other?) and I'm trying to understand the response from @MichaelScofield : "but I think you should be able to dependent on table in meta-client. table does not dependent on meta-client" what does it mean by table does not dependent on meta-client? (Question: 1. what is the table you are referring to here? And do you mean "table should be able to depends on meta-client, but meta-client doesn't need to depends on table? meaning meta-client is imported by table")

[MichaelScofield]

@qstommyshu table crate should be logically lower level than meta-client. Let's reuse the TableReference struct we already have, and forget about TableFullName. All in all, we just want to unify the table naming usages -- TableName, (catalog, schema, table) or TableReference -- to a standardized, canonical way.