Skip to content

Commit

Permalink
[apache#3698] refactor (API): Refactor client side RelationalCatalog …
Browse files Browse the repository at this point in the history 
…to use relative path in NameIdentifier (apache#3789)

### What changes were proposed in this pull request?

Currently, in the RelationalCatalog.java, the methods like "loadTable",
"createTable", "updateTable" all need a NameIdentifier parameter, which
needs to be a fully-qualified (metalake.catalog.schema.table) name. But
the "metalake" and "catalog" are not needed, as they already be provided
when load the catalog. To make the API clear and easier to use, we will
change it to use a relative NameIdentifier object (which is
"schema.table") as the table's ID, so that the user doesn't need to
provide the metalake and catalog names repeatedly.

Please note, this only affects the client side. 

### Why are the changes needed?

To make the API simple and easy to understand.

Fix: apache#3698

### Does this PR introduce _any_ user-facing change?

No behavior change, just method parameter.

### How was this patch tested?

No introduce new class or method, so the change will be covered by all
existing test cases.
  • Loading branch information
@shaofengshi
shaofengshi authored Jun 24, 2024
1 parent ca9f220 commit c2ae2a3
Show file tree
Hide file tree
Showing 24 changed files with 395 additions and 593 deletions.
131 changes: 47 additions & 84 deletions ...e/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java
View file

Large diffs are not rendered by default.

6 changes: 2 additions & 4 deletions ...java/com/datastrato/gravitino/catalog/hive/integration/test/HiveUserAuthenticationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ public void testUserAuthentication() {
() -> catalog.asSchemas().createSchema(SCHEMA_NAME, "comment", ImmutableMap.of()));

// Create table
NameIdentifier tableNameIdentifier =
NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, TABLE_NAME);
NameIdentifier tableNameIdentifier = NameIdentifier.of(SCHEMA_NAME, TABLE_NAME);
catalog
.asTableCatalog()
.createTable(
Expand All @@ -246,8 +245,7 @@ public void testUserAuthentication() {

// Now try to alter the table
catalog.asTableCatalog().alterTable(tableNameIdentifier, TableChange.rename("new_table"));
NameIdentifier newTableIdentifier =
NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, "new_table");
NameIdentifier newTableIdentifier = NameIdentifier.of(SCHEMA_NAME, "new_table");

// Old table name should not exist
Assertions.assertFalse(catalog.asTableCatalog().tableExists(tableNameIdentifier));
Expand Down
23 changes: 8 additions & 15 deletions .../test/java/com/datastrato/gravitino/catalog/hive/integration/test/ProxyCatalogHiveIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,8 @@ public void testOperateSchema() throws Exception {
String schemaName = GravitinoITUtils.genRandomName(SCHEMA_PREFIX);
String anotherSchemaName = GravitinoITUtils.genRandomName(SCHEMA_PREFIX);

NameIdentifier ident = NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName);

String comment = "comment";
createSchema(schemaName, ident, comment);
createSchema(schemaName, comment);

Database db = hiveClientPool.run(client -> client.getDatabase(schemaName));
Assertions.assertEquals(EXPECT_USER, db.getOwnerName());
Expand Down Expand Up @@ -163,14 +161,11 @@ public void testOperateTable() throws Exception {
String tableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);
String anotherTableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);

NameIdentifier ident = NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName);
NameIdentifier nameIdentifier =
NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName, tableName);
NameIdentifier anotherNameIdentifier =
NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName, anotherTableName);
NameIdentifier nameIdentifier = NameIdentifier.of(schemaName, tableName);
NameIdentifier anotherNameIdentifier = NameIdentifier.of(schemaName, anotherTableName);

String comment = "comment";
createSchema(schemaName, ident, comment);
createSchema(schemaName, comment);

Table createdTable =
catalog
Expand Down Expand Up @@ -200,7 +195,7 @@ public void testOperateTable() throws Exception {
Assertions.assertTrue(e.getMessage().contains("AccessControlException Permission denied"));
}

private static void createSchema(String schemaName, NameIdentifier ident, String comment) {
private static void createSchema(String schemaName, String comment) {
Map<String, String> properties = Maps.newHashMap();
properties.put("key1", "val1");
properties.put("key2", "val2");
Expand All @@ -211,7 +206,7 @@ private static void createSchema(String schemaName, NameIdentifier ident, String
containerSuite.getHiveContainer().getContainerIpAddress(),
HiveContainer.HDFS_DEFAULTFS_PORT,
schemaName.toLowerCase()));
catalog.asSchemas().createSchema(ident.name(), comment, properties);
catalog.asSchemas().createSchema(schemaName, comment, properties);
}

@Test
Expand All @@ -221,12 +216,10 @@ public void testOperatePartition() throws Exception {
String schemaName = GravitinoITUtils.genRandomName(SCHEMA_PREFIX);
String tableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);

NameIdentifier ident = NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName);
NameIdentifier nameIdentifier =
NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName, tableName);
NameIdentifier nameIdentifier = NameIdentifier.of(schemaName, tableName);

String comment = "comment";
createSchema(schemaName, ident, comment);
createSchema(schemaName, comment);

// create a partitioned table
Column[] columns = createColumns();
Expand Down
24 changes: 9 additions & 15 deletions ...src/test/java/com/datastrato/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ void testDropDorisSchema() {
catalog
.asTableCatalog()
.createTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName),
NameIdentifier.of(schemaName, tableName),
createColumns(),
"Created by gravitino client",
createTableProperties(),
Expand Down Expand Up @@ -322,8 +322,7 @@ void testSchemaWithIllegalName() {
@Test
void testDorisTableBasicOperation() {
// create a table
NameIdentifier tableIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
Column[] columns = createColumns();

Distribution distribution = createDistribution();
Expand Down Expand Up @@ -357,8 +356,7 @@ void testDorisTableBasicOperation() {
// rename table
String newTableName = GravitinoITUtils.genRandomName("new_table_name");
tableCatalog.alterTable(tableIdentifier, TableChange.rename(newTableName));
NameIdentifier newTableIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, newTableName);
NameIdentifier newTableIdentifier = NameIdentifier.of(schemaName, newTableName);
Table renamedTable = tableCatalog.loadTable(newTableIdentifier);
ITUtils.assertionsTableInfo(
newTableName, table_comment, Arrays.asList(columns), properties, indexes, renamedTable);
Expand Down Expand Up @@ -479,8 +477,7 @@ void testDorisIllegalTableName() {
@Test
void testAlterDorisTable() {
// create a table
NameIdentifier tableIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
Column[] columns = createColumns();

Distribution distribution = createDistribution();
Expand Down Expand Up @@ -572,8 +569,7 @@ void testAlterDorisTable() {
void testDorisIndex() {
String tableName = GravitinoITUtils.genRandomName("test_add_index");

NameIdentifier tableIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
Column[] columns = createColumns();

Distribution distribution = createDistribution();
Expand All @@ -593,7 +589,7 @@ void testDorisIndex() {

// add index test.
tableCatalog.alterTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName),
NameIdentifier.of(schemaName, tableName),
TableChange.addIndex(
Index.IndexType.PRIMARY_KEY, "k1_index", new String[][] {{DORIS_COL_NAME1}}));

Expand All @@ -605,14 +601,13 @@ void testDorisIndex() {
assertEquals(
1,
tableCatalog
.loadTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName))
.loadTable(NameIdentifier.of(schemaName, tableName))
.index()
.length));

// delete index and add new column and index.
tableCatalog.alterTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName),
NameIdentifier.of(schemaName, tableName),
TableChange.deleteIndex("k1_index", true),
TableChange.addIndex(
Index.IndexType.PRIMARY_KEY, "k2_index", new String[][] {{DORIS_COL_NAME2}}));
Expand All @@ -625,8 +620,7 @@ void testDorisIndex() {
assertEquals(
1,
tableCatalog
.loadTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName))
.loadTable(NameIdentifier.of(schemaName, tableName))
.index()
.length));
}
Expand Down
8 changes: 3 additions & 5 deletions ...est/java/com/datastrato/gravitino/catalog/mysql/integration/test/AuditCatalogMysqlIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void testAuditTable() throws Exception {
catalog
.asTableCatalog()
.createTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName),
NameIdentifier.of(schemaName, tableName),
new Column[] {col1},
"comment",
properties);
Expand All @@ -123,14 +123,12 @@ public void testAuditTable() throws Exception {
catalog
.asTableCatalog()
.alterTable(
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName),
NameIdentifier.of(schemaName, tableName),
TableChange.addColumn(new String[] {"col_4"}, Types.StringType.get()));
Assertions.assertEquals(expectUser, table.auditInfo().creator());
Assertions.assertEquals(expectUser, table.auditInfo().lastModifier());

catalog
.asTableCatalog()
.dropTable(NameIdentifier.of(metalakeName, catalogName, schemaName, tableName));
catalog.asTableCatalog().dropTable(NameIdentifier.of(schemaName, tableName));
catalog.asSchemas().dropSchema(schemaName, true);
metalake.dropCatalog(catalogName);
}
Expand Down
Loading

0 comments on commit c2ae2a3

Please sign in to comment.