Skip to content

Commit

Permalink
Addressed review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@nk1506
nk1506 committed Oct 28, 2023
1 parent 6a34141 commit b3c8edc
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 286 deletions.
45 changes: 37 additions & 8 deletions core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,15 @@ public void createTableViaTransactionThatAlreadyExistsAsView() {
assertThat(catalog().viewExists(viewIdentifier)).as("View should exist").isTrue();

assertThatThrownBy(transaction::commitTransaction)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageStartingWith("View with same name already exists: ns.view");
.satisfiesAnyOf(
throwable ->
assertThat(throwable)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageStartingWith("Table already exists: ns.view"),
throwable ->
assertThat(throwable)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageStartingWith("View with same name already exists: ns.view"));
}

@Test
Expand Down Expand Up @@ -400,8 +407,15 @@ public void replaceTableViaTransactionThatAlreadyExistsAsView() {
.buildTable(viewIdentifier, SCHEMA)
.replaceTransaction()
.commitTransaction())
.isInstanceOf(NoSuchTableException.class)
.hasMessageStartingWith("Table does not exist: ns.view");
.satisfiesAnyOf(
throwable ->
assertThat(throwable)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageStartingWith("View with same name already exists: ns.view"),
throwable ->
assertThat(throwable)
.isInstanceOf(NoSuchTableException.class)
.hasMessageStartingWith("Table does not exist: ns.view"));
}

@Test
Expand Down Expand Up @@ -465,8 +479,15 @@ public void replaceViewThatAlreadyExistsAsTable() {
.withDefaultNamespace(tableIdentifier.namespace())
.withQuery("spark", "select * from ns.tbl")
.replace())
.isInstanceOf(NoSuchViewException.class)
.hasMessageStartingWith("View does not exist: ns.table");
.satisfiesAnyOf(
throwable ->
assertThat(throwable)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageStartingWith("Table with same name already exists: ns.table"),
throwable ->
assertThat(throwable)
.isInstanceOf(NoSuchViewException.class)
.hasMessageStartingWith("View does not exist: ns.table"));
}

@Test
Expand Down Expand Up @@ -715,8 +736,16 @@ public void renameTableTargetAlreadyExistsAsView() {
assertThat(catalog().viewExists(viewIdentifier)).as("View should exist").isTrue();

assertThatThrownBy(() -> tableCatalog().renameTable(tableIdentifier, viewIdentifier))
.isInstanceOf(AlreadyExistsException.class)
.hasMessageContaining("Cannot rename ns.table to ns.view. View already exists");
.satisfiesAnyOf(
throwable ->
assertThat(throwable)
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("new table ns.view already exists"),
throwable ->
assertThat(throwable)
.isInstanceOf(AlreadyExistsException.class)
.hasMessageContaining(
"Cannot rename ns.table to ns.view. View already exists"));
}

@Test
Expand Down
96 changes: 38 additions & 58 deletions hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configurable;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.conf.HiveConf;
Expand Down Expand Up @@ -124,43 +126,13 @@ public void initialize(String inputName, Map<String, String> properties) {

@Override
public List<TableIdentifier> listTables(Namespace namespace) {
Preconditions.checkArgument(
isValidateNamespace(namespace), "Missing database in namespace: %s", namespace);
String database = namespace.level(0);

try {
List<String> tableNames = clients.run(client -> client.getAllTables(database));
List<TableIdentifier> tableIdentifiers;

if (listAllTables) {
tableIdentifiers =
tableNames.stream()
.map(t -> TableIdentifier.of(namespace, t))
.collect(Collectors.toList());
return listContents(namespace, null, table -> true);
} else {
List<Table> tableObjects =
clients.run(client -> client.getTableObjectsByName(database, tableNames));
tableIdentifiers =
tableObjects.stream()
.filter(
table ->
table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.name())
&& table.getParameters() != null
&& BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
.equalsIgnoreCase(
table
.getParameters()
.get(BaseMetastoreTableOperations.TABLE_TYPE_PROP)))
.map(table -> TableIdentifier.of(namespace, table.getTableName()))
.collect(Collectors.toList());
return listContents(namespace, TableType.EXTERNAL_TABLE.name(), icebergPredicate());
}

LOG.debug(
"Listing of namespace: {} resulted in the following tables: {}",
namespace,
tableIdentifiers);
return tableIdentifiers;

} catch (UnknownDBException e) {
throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace);

Expand Down Expand Up @@ -315,33 +287,8 @@ public boolean dropView(TableIdentifier identifier) {

@Override
public List<TableIdentifier> listViews(Namespace namespace) {
Preconditions.checkArgument(
isValidateNamespace(namespace), "Missing database in namespace: %s", namespace);
String database = namespace.level(0);

try {
List<String> tableNames =
clients.run(client -> client.getTables(database, "*", TableType.VIRTUAL_VIEW));
List<Table> tableObjects =
clients.run(client -> client.getTableObjectsByName(database, tableNames));
List<TableIdentifier> tableIdentifiers =
tableObjects.stream()
.filter(
table ->
table.getParameters() != null
&& BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(
table
.getParameters()
.get(BaseMetastoreTableOperations.TABLE_TYPE_PROP)))
.map(table -> TableIdentifier.of(namespace, table.getTableName()))
.collect(Collectors.toList());

LOG.debug(
"Listing of namespace: {} resulted in the following views: {}",
namespace,
tableIdentifiers);
return tableIdentifiers;

return listContents(namespace, TableType.VIRTUAL_VIEW.name(), icebergPredicate());
} catch (UnknownDBException e) {
throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace);

Expand All @@ -354,6 +301,39 @@ public List<TableIdentifier> listViews(Namespace namespace) {
}
}

private List<TableIdentifier> listContents(
Namespace namespace, String tableType, Predicate<Table> tablePredicate)
throws TException, InterruptedException {
Preconditions.checkArgument(
isValidateNamespace(namespace), "Missing database in namespace: %s", namespace);
String database = namespace.level(0);
List<String> tableNames =
StringUtils.isNotEmpty(tableType)
? clients.run(client -> client.getTables(database, "*", TableType.valueOf(tableType)))
: clients.run(client -> client.getAllTables(database));
List<Table> tableObjects =
clients.run(client -> client.getTableObjectsByName(database, tableNames));
List<TableIdentifier> tableIdentifiers =
tableObjects.stream()
.filter(tablePredicate)
.map(table -> TableIdentifier.of(namespace, table.getTableName()))
.collect(Collectors.toList());

LOG.debug(
"Listing of namespace: {} for table type {} resulted in the following: {}",
namespace,
tableType,
tableIdentifiers);
return tableIdentifiers;
}

private Predicate<Table> icebergPredicate() {
return table ->
table.getParameters() != null
&& BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(
table.getParameters().get(BaseMetastoreTableOperations.TABLE_TYPE_PROP));
}

@Override
@SuppressWarnings("FormatStringAnnotation")
public void renameView(TableIdentifier from, TableIdentifier originalTo) {
Expand Down
73 changes: 0 additions & 73 deletions hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreSetup.java
View file

This file was deleted.

Loading

0 comments on commit b3c8edc

Please sign in to comment.