Skip to content

Commit

Permalink
[core] Rename some methods of AbstractCatalog for location
Browse files Browse the repository at this point in the history
  • Loading branch information
@JingsongLi
JingsongLi committed Nov 27, 2023
1 parent 60d7ca1 commit 34a619f
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 35 deletions.
17 changes: 8 additions & 9 deletions paimon-core/src/main/java/org/apache/paimon/catalog/AbstractCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ private FileStoreTable getDataTable(Identifier identifier) throws TableNotExistE
}

@VisibleForTesting
public Path databasePath(String database) {
return databasePath(warehouse(), database);
public Path newDatabasePath(String database) {
return newDatabasePath(warehouse(), database);
}

Map<String, Map<String, Path>> allTablePaths() {
Expand All @@ -306,9 +306,7 @@ Map<String, Map<String, Path>> allTablePaths() {
Map<String, Path> tableMap =
allPaths.computeIfAbsent(database, d -> new HashMap<>());
for (String table : listTables(database)) {
tableMap.put(
table,
dataTableLocation(warehouse(), Identifier.create(database, table)));
tableMap.put(table, getDataTableLocation(Identifier.create(database, table)));
}
}
return allPaths;
Expand All @@ -328,7 +326,7 @@ protected abstract TableSchema getDataTableSchema(Identifier identifier)

@VisibleForTesting
public Path getDataTableLocation(Identifier identifier) {
return dataTableLocation(warehouse(), identifier);
return newTableLocation(warehouse(), identifier);
}

private static boolean isSpecifiedSystemTable(Identifier identifier) {
Expand Down Expand Up @@ -362,18 +360,19 @@ private String[] tableAndSystemName(Identifier identifier) {
return splits;
}

public static Path dataTableLocation(String warehouse, Identifier identifier) {
public static Path newTableLocation(String warehouse, Identifier identifier) {
if (isSpecifiedSystemTable(identifier)) {
throw new IllegalArgumentException(
String.format(
"Table name[%s] cannot contain '%s' separator",
identifier.getObjectName(), SYSTEM_TABLE_SPLITTER));
}
return new Path(
databasePath(warehouse, identifier.getDatabaseName()), identifier.getObjectName());
newDatabasePath(warehouse, identifier.getDatabaseName()),
identifier.getObjectName());
}

public static Path databasePath(String warehouse, String database) {
public static Path newDatabasePath(String warehouse, String database) {
return new Path(warehouse, database + DB_SUFFIX);
}

Expand Down
8 changes: 4 additions & 4 deletions paimon-core/src/main/java/org/apache/paimon/catalog/FileSystemCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,23 @@ public List<String> listDatabases() {

@Override
protected boolean databaseExistsImpl(String databaseName) {
return uncheck(() -> fileIO.exists(databasePath(databaseName)));
return uncheck(() -> fileIO.exists(newDatabasePath(databaseName)));
}

@Override
protected void createDatabaseImpl(String name) {
uncheck(() -> fileIO.mkdirs(databasePath(name)));
uncheck(() -> fileIO.mkdirs(newDatabasePath(name)));
}

@Override
protected void dropDatabaseImpl(String name) {
uncheck(() -> fileIO.delete(databasePath(name), true));
uncheck(() -> fileIO.delete(newDatabasePath(name), true));
}

@Override
protected List<String> listTablesImpl(String databaseName) {
List<String> tables = new ArrayList<>();
for (FileStatus status : uncheck(() -> fileIO.listStatus(databasePath(databaseName)))) {
for (FileStatus status : uncheck(() -> fileIO.listStatus(newDatabasePath(databaseName)))) {
if (status.isDir() && tableExists(status.getPath())) {
tables.add(status.getPath().getName());
}
Expand Down
20 changes: 10 additions & 10 deletions paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/hive/HiveCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,13 @@ protected boolean databaseExistsImpl(String databaseName) {
@Override
protected void createDatabaseImpl(String name) {
try {
client.createDatabase(convertToDatabase(name));
locationHelper.createPathIfRequired(databasePath(name), fileIO);
Path databasePath = newDatabasePath(name);
locationHelper.createPathIfRequired(databasePath, fileIO);

Database database = new Database();
database.setName(name);
locationHelper.specifyDatabaseLocation(databasePath, database);
client.createDatabase(database);
} catch (TException | IOException e) {
throw new RuntimeException("Failed to create database " + name, e);
}
Expand All @@ -218,7 +223,9 @@ protected void createDatabaseImpl(String name) {
@Override
protected void dropDatabaseImpl(String name) {
try {
locationHelper.dropPathIfRequired(databasePath(name), fileIO);
Database database = client.getDatabase(name);
String location = locationHelper.getDatabaseLocation(database);
locationHelper.dropPathIfRequired(new Path(location), fileIO);
client.dropDatabase(name, true, false, true);
} catch (TException | IOException e) {
throw new RuntimeException("Failed to drop database " + name, e);
Expand Down Expand Up @@ -421,13 +428,6 @@ private void checkIdentifierUpperCase(Identifier identifier) {
identifier.getObjectName()));
}

private Database convertToDatabase(String name) {
Database database = new Database();
database.setName(name);
locationHelper.specifyDatabaseLocation(databasePath(name), database);
return database;
}

private Table newHmsTable(Identifier identifier, Map<String, String> tableParameters) {
long currentTimeMillis = System.currentTimeMillis();
TableType tableType =
Expand Down
2 changes: 1 addition & 1 deletion ...ive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void preCreateTable(Table table) throws MetaException {
org.apache.hadoop.fs.Path hadoopPath =
getDnsPath(new org.apache.hadoop.fs.Path(warehouse), conf);
warehouse = hadoopPath.toUri().toString();
location = AbstractCatalog.dataTableLocation(warehouse, identifier).toUri().toString();
location = AbstractCatalog.newTableLocation(warehouse, identifier).toUri().toString();
table.getSd().setLocation(location);
}

Expand Down
18 changes: 9 additions & 9 deletions .../paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void testCreateExternalTableWithPaimonTable() throws Exception {
Maps.newHashMap(),
"");
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
new SchemaManager(LocalFileIO.create(), tablePath).createTable(schema);

// Create hive external table
Expand Down Expand Up @@ -156,7 +156,7 @@ public void testCreateTableUsePartitionedBy() {

// check the paimon table schema
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Optional<TableSchema> tableSchema =
new SchemaManager(LocalFileIO.create(), tablePath).latest();
assertThat(tableSchema).isPresent();
Expand Down Expand Up @@ -212,7 +212,7 @@ public void testLowerTableName() throws Catalog.TableNotExistException {
}
// check the paimon table name and schema
Identifier identifier = Identifier.create(DATABASE_TEST, tableName.toLowerCase());
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Options conf = new Options();
conf.set(CatalogOptions.WAREHOUSE, path);
CatalogContext catalogContext = CatalogContext.create(conf);
Expand Down Expand Up @@ -277,7 +277,7 @@ public void testLowerDBName() throws Catalog.TableNotExistException {

// check the paimon db name、table name and schema
Identifier identifier = Identifier.create(upperDB.toLowerCase(), tableName.toLowerCase());
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Options conf = new Options();
conf.set(CatalogOptions.WAREHOUSE, path);
CatalogContext catalogContext = CatalogContext.create(conf);
Expand Down Expand Up @@ -322,7 +322,7 @@ public void testCreateTableWithPrimaryKey() {

// check the paimon table schema
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Optional<TableSchema> tableSchema =
new SchemaManager(LocalFileIO.create(), tablePath).latest();
assertThat(tableSchema).isPresent();
Expand Down Expand Up @@ -364,7 +364,7 @@ public void testCreateTableWithPartition() {

// check the paimon table schema
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Optional<TableSchema> tableSchema =
new SchemaManager(LocalFileIO.create(), tablePath).latest();
assertThat(tableSchema).isPresent();
Expand Down Expand Up @@ -408,7 +408,7 @@ public void testCreateTableSpecifyProperties() {

// check the paimon table schema
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
Optional<TableSchema> tableSchema =
new SchemaManager(LocalFileIO.create(), tablePath).latest();
assertThat(tableSchema).isPresent();
Expand Down Expand Up @@ -456,7 +456,7 @@ public void testCreateTableFailing() throws Exception {
Maps.newHashMap(),
"");
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
new SchemaManager(LocalFileIO.create(), tablePath).createTable(schema);

String hiveSql =
Expand Down Expand Up @@ -500,7 +500,7 @@ public void testCreateTableFailing() throws Exception {
} catch (Exception ignore) {
} finally {
Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
Path tablePath = AbstractCatalog.newTableLocation(path, identifier);
boolean isPresent =
new SchemaManager(LocalFileIO.create(), tablePath).latest().isPresent();
Assertions.assertThat(isPresent).isFalse();
Expand Down
4 changes: 2 additions & 2 deletions ...e/paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/HiveLocationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void testCatalogDBLocation() throws Exception {
catalog.createDatabase(db, true);
assertThat(hmsClient.getDatabase(db)).isNotNull();

Path actual = catalog.databasePath(db);
Path actual = catalog.newDatabasePath(db);
Path expected = new Path(this.objectStorepath + "/" + db + ".db");
assertThat(fileIO.exists(expected)).isTrue();
assertThat(actual).isEqualTo(expected);
Expand Down Expand Up @@ -256,7 +256,7 @@ public void testRWIT() {

Identifier identifier = Identifier.create(dbName, tableName);
String location =
AbstractCatalog.dataTableLocation(warehouse, identifier).toUri().toString();
AbstractCatalog.newTableLocation(warehouse, identifier).toUri().toString();

String createTableSqlStr =
getCreateTableSqlStr(tableName, location, locationInProperties);
Expand Down

0 comments on commit 34a619f

Please sign in to comment.