Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Add Table.uuid method #4213

Merged
merged 6 commits into from
Nov 13, 2024
Merged

[core] Add Table.uuid method #4213

merged 6 commits into from
Nov 13, 2024

Conversation

JingsongLi
Copy link
Contributor

@JingsongLi JingsongLi commented Sep 19, 2024

Purpose

Add Table.uuid method for table uniq id, this can be used like some RBAC support.

  • for filesystem catalog, uuid is db + tableName + schema-createTime.
  • for hive catalog, uuid is db + tableName + createTime.

Tests

API and Format

Documentation

if (catalogEnvironment.uuid() != null) {
return catalogEnvironment.uuid();
}
return fullName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should at least log warning or add a setting to enabling simply use fullName() as uuid()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must let this method work at least. Log will produce many logs for filesystem catalog.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can find better implementation for non-hive catalog tables. And system tables.

Copy link
Contributor

@adrian-wang adrian-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 2 small questions

if (catalogEnvironment.uuid() != null) {
return catalogEnvironment.uuid();
}
long earliestCreationTime = schemaManager().earliestCreationTime();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this earliestCreationTime change as we do compaction or snapshot expire?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never, actually we do not clean schema files, so the first schema file (schema-0) will always exist, and immutable.

So this earliestCreationTime should always be the same.

catalog.createTable(identifier, DEFAULT_TABLE_SCHEMA, false);
Table table = catalog.getTable(identifier);
String uuid = table.uuid();
assertThat(uuid).startsWith(identifier.getFullName() + ".");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail those non-FileStoreTable, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@adrian-wang
Copy link
Contributor

LGTM, +1, let's merge this!

@JingsongLi JingsongLi merged commit 72c25d5 into apache:master Nov 13, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants