Skip to content

Commit

Permalink
[core] Delete useless methods in Catalog (apache#4468)
Browse files Browse the repository at this point in the history
  • Loading branch information
JingsongLi authored and guanshi committed Nov 7, 2024
1 parent c65ff11 commit c9ecf27
Show file tree
Hide file tree
Showing 19 changed files with 288 additions and 290 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ public FileIO fileIO() {
return fileIO;
}

@Override
public Optional<CatalogLockFactory> lockFactory() {
if (!lockEnabled()) {
return Optional.empty();
Expand All @@ -118,7 +117,6 @@ public Optional<CatalogLockFactory> defaultLockFactory() {
return Optional.empty();
}

@Override
public Optional<CatalogLockContext> lockContext() {
return Optional.of(CatalogLockContext.fromOptions(catalogOptions));
}
Expand All @@ -136,26 +134,26 @@ public boolean allowUpperCase() {
public void createDatabase(String name, boolean ignoreIfExists, Map<String, String> properties)
throws DatabaseAlreadyExistException {
checkNotSystemDatabase(name);
if (databaseExists(name)) {
try {
getDatabase(name);
if (ignoreIfExists) {
return;
}
throw new DatabaseAlreadyExistException(name);
} catch (DatabaseNotExistException ignored) {
}
createDatabaseImpl(name, properties);
}

@Override
public Map<String, String> loadDatabaseProperties(String name)
throws DatabaseNotExistException {
public Database getDatabase(String name) throws DatabaseNotExistException {
if (isSystemDatabase(name)) {
return Collections.emptyMap();
return Database.of(name);
}
return loadDatabasePropertiesImpl(name);
return getDatabaseImpl(name);
}

protected abstract Map<String, String> loadDatabasePropertiesImpl(String name)
throws DatabaseNotExistException;
protected abstract Database getDatabaseImpl(String name) throws DatabaseNotExistException;

@Override
public void createPartition(Identifier identifier, Map<String, String> partitionSpec)
Expand Down Expand Up @@ -211,7 +209,9 @@ public List<PartitionEntry> listPartitions(Identifier identifier)
public void dropDatabase(String name, boolean ignoreIfNotExists, boolean cascade)
throws DatabaseNotExistException, DatabaseNotEmptyException {
checkNotSystemDatabase(name);
if (!databaseExists(name)) {
try {
getDatabase(name);
} catch (DatabaseNotExistException e) {
if (ignoreIfNotExists) {
return;
}
Expand All @@ -232,9 +232,9 @@ public List<String> listTables(String databaseName) throws DatabaseNotExistExcep
if (isSystemDatabase(databaseName)) {
return SystemTableLoader.loadGlobalTableNames();
}
if (!databaseExists(databaseName)) {
throw new DatabaseNotExistException(databaseName);
}

// check db exists
getDatabase(databaseName);

return listTablesImpl(databaseName).stream().sorted().collect(Collectors.toList());
}
Expand All @@ -247,7 +247,9 @@ public void dropTable(Identifier identifier, boolean ignoreIfNotExists)
checkNotBranch(identifier, "dropTable");
checkNotSystemTable(identifier, "dropTable");

if (!tableExists(identifier)) {
try {
getTable(identifier);
} catch (TableNotExistException e) {
if (ignoreIfNotExists) {
return;
}
Expand All @@ -268,15 +270,16 @@ public void createTable(Identifier identifier, Schema schema, boolean ignoreIfEx
validateFieldNameCaseInsensitive(schema.rowType().getFieldNames());
validateAutoCreateClose(schema.options());

if (!databaseExists(identifier.getDatabaseName())) {
throw new DatabaseNotExistException(identifier.getDatabaseName());
}
// check db exists
getDatabase(identifier.getDatabaseName());

if (tableExists(identifier)) {
try {
getTable(identifier);
if (ignoreIfExists) {
return;
}
throw new TableAlreadyExistException(identifier);
} catch (TableNotExistException ignored) {
}

copyTableDefaultOptions(schema.options());
Expand All @@ -299,15 +302,19 @@ public void renameTable(Identifier fromTable, Identifier toTable, boolean ignore
checkNotSystemTable(toTable, "renameTable");
validateIdentifierNameCaseInsensitive(toTable);

if (!tableExists(fromTable)) {
try {
getTable(fromTable);
} catch (TableNotExistException e) {
if (ignoreIfNotExists) {
return;
}
throw new TableNotExistException(fromTable);
}

if (tableExists(toTable)) {
try {
getTable(toTable);
throw new TableAlreadyExistException(toTable);
} catch (TableNotExistException ignored) {
}

renameTableImpl(fromTable, toTable);
Expand All @@ -323,7 +330,9 @@ public void alterTable(
validateIdentifierNameCaseInsensitive(identifier);
validateFieldNameCaseInsensitiveInSchemaChange(changes);

if (!tableExists(identifier)) {
try {
getTable(identifier);
} catch (TableNotExistException e) {
if (ignoreIfNotExists) {
return;
}
Expand Down Expand Up @@ -452,6 +461,12 @@ public Map<String, Map<String, Path>> allTablePaths() {
protected abstract TableSchema getDataTableSchema(Identifier identifier)
throws TableNotExistException;

/** Get metastore client factory for the table specified by {@code identifier}. */
protected Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier identifier)
throws TableNotExistException {
return Optional.empty();
}

@Override
public Path getTableLocation(Identifier identifier) {
return new Path(newDatabasePath(identifier.getDatabaseName()), identifier.getTableName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class CachingCatalog extends DelegateCatalog {

private static final Logger LOG = LoggerFactory.getLogger(CachingCatalog.class);

protected final Cache<String, Map<String, String>> databaseCache;
protected final Cache<String, Database> databaseCache;
protected final Cache<Identifier, Table> tableCache;
@Nullable protected final SegmentsCache<Path> manifestCache;

Expand Down Expand Up @@ -159,16 +159,15 @@ public static Catalog tryToCreate(Catalog catalog, Options options) {
}

@Override
public Map<String, String> loadDatabaseProperties(String databaseName)
throws DatabaseNotExistException {
Map<String, String> properties = databaseCache.getIfPresent(databaseName);
if (properties != null) {
return properties;
public Database getDatabase(String databaseName) throws DatabaseNotExistException {
Database database = databaseCache.getIfPresent(databaseName);
if (database != null) {
return database;
}

properties = super.loadDatabaseProperties(databaseName);
databaseCache.put(databaseName, properties);
return properties;
database = super.getDatabase(databaseName);
databaseCache.put(databaseName, database);
return database;
}

@Override
Expand Down
71 changes: 6 additions & 65 deletions paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.paimon.fs.FileIO;
import org.apache.paimon.fs.Path;
import org.apache.paimon.manifest.PartitionEntry;
import org.apache.paimon.metastore.MetastoreClient;
import org.apache.paimon.schema.Schema;
import org.apache.paimon.schema.SchemaChange;
import org.apache.paimon.table.Table;
Expand All @@ -33,7 +32,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import static org.apache.paimon.options.OptionsUtils.convertToPropertiesPrefixKey;
Expand Down Expand Up @@ -72,45 +70,13 @@ public interface Catalog extends AutoCloseable {

FileIO fileIO();

/**
* Get lock factory from catalog. Lock is used to support multiple concurrent writes on the
* object store.
*/
Optional<CatalogLockFactory> lockFactory();

/** Get lock context for lock factory to create a lock. */
default Optional<CatalogLockContext> lockContext() {
return Optional.empty();
}

/** Get metastore client factory for the table specified by {@code identifier}. */
default Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier identifier)
throws TableNotExistException {
return Optional.empty();
}

/**
* Get the names of all databases in this catalog.
*
* @return a list of the names of all databases
*/
List<String> listDatabases();

/**
* Check if a database exists in this catalog.
*
* @param databaseName Name of the database
* @return true if the given database exists in the catalog false otherwise
*/
default boolean databaseExists(String databaseName) {
try {
loadDatabaseProperties(databaseName);
return true;
} catch (DatabaseNotExistException e) {
return false;
}
}

/**
* Create a database, see {@link Catalog#createDatabase(String name, boolean ignoreIfExists, Map
* properties)}.
Expand All @@ -135,13 +101,13 @@ void createDatabase(String name, boolean ignoreIfExists, Map<String, String> pro
throws DatabaseAlreadyExistException;

/**
* Load database properties.
* Return a {@link Database} identified by the given name.
*
* @param name Database name
* @return The requested database's properties
* @return The requested {@link Database}
* @throws DatabaseNotExistException if the requested database does not exist
*/
Map<String, String> loadDatabaseProperties(String name) throws DatabaseNotExistException;
Database getDatabase(String name) throws DatabaseNotExistException;

/**
* Drop a database.
Expand Down Expand Up @@ -186,20 +152,6 @@ void dropDatabase(String name, boolean ignoreIfNotExists, boolean cascade)
*/
List<String> listTables(String databaseName) throws DatabaseNotExistException;

/**
* Check if a table exists in this catalog.
*
* @param identifier Path of the table
* @return true if the given table exists in the catalog false otherwise
*/
default boolean tableExists(Identifier identifier) {
try {
return getTable(identifier) != null;
} catch (TableNotExistException e) {
return false;
}
}

/**
* Drop a table.
*
Expand Down Expand Up @@ -273,6 +225,9 @@ default void invalidateTable(Identifier identifier) {}
/**
* Create the partition of the specify table.
*
* <p>Only catalog with metastore can support this method, and only table with
* 'metastore.partitioned-table' can support this method.
*
* @param identifier path of the table to drop partition
* @param partitionSpec the partition to be created
* @throws TableNotExistException if the table does not exist
Expand Down Expand Up @@ -315,20 +270,6 @@ default void alterTable(Identifier identifier, SchemaChange change, boolean igno
alterTable(identifier, Collections.singletonList(change), ignoreIfNotExists);
}

/**
* Check if a view exists in this catalog.
*
* @param identifier Path of the view
* @return true if the given view exists in the catalog false otherwise
*/
default boolean viewExists(Identifier identifier) {
try {
return getView(identifier) != null;
} catch (ViewNotExistException e) {
return false;
}
}

/**
* Return a {@link View} identified by the given {@link Identifier}.
*
Expand Down
82 changes: 82 additions & 0 deletions paimon-core/src/main/java/org/apache/paimon/catalog/Database.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.paimon.catalog;

import org.apache.paimon.annotation.Public;

import javax.annotation.Nullable;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

/**
* Interface of a database in a catalog.
*
* @since 1.0
*/
@Public
public interface Database {

/** A name to identify this database. */
String name();

/** Options of this database. */
Map<String, String> options();

/** Optional comment of this database. */
Optional<String> comment();

static Database of(String name, Map<String, String> options, @Nullable String comment) {
return new DatabaseImpl(name, options, comment);
}

static Database of(String name) {
return new DatabaseImpl(name, new HashMap<>(), null);
}

/** Implementation of {@link Database}. */
class DatabaseImpl implements Database {

private final String name;
private final Map<String, String> options;
@Nullable private final String comment;

public DatabaseImpl(String name, Map<String, String> options, @Nullable String comment) {
this.name = name;
this.options = options;
this.comment = comment;
}

@Override
public String name() {
return name;
}

@Override
public Map<String, String> options() {
return options;
}

@Override
public Optional<String> comment() {
return Optional.ofNullable(comment);
}
}
}
Loading

0 comments on commit c9ecf27

Please sign in to comment.