Skip to content

Commit

Permalink
Extend testListTables with other relation types
Browse files Browse the repository at this point in the history 
TrinoCatalog.listTables returns not only iceberg tables but also
other relations like views, materialized view or non-iceberg tables.
  • Loading branch information
@lukasz-stec @raunaqmorarka
lukasz-stec authored and raunaqmorarka committed Dec 23, 2024
1 parent 8dfa5ab commit 563ba35
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 3 deletions.
90 changes: 88 additions & 2 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
import io.trino.plugin.base.util.AutoCloseableCloser;
import io.trino.plugin.hive.NodeVersion;
import io.trino.plugin.iceberg.CommitTaskData;
import io.trino.plugin.iceberg.IcebergFileFormat;
import io.trino.plugin.iceberg.IcebergMetadata;
import io.trino.plugin.iceberg.TableStatisticsWriter;
import io.trino.spi.TrinoException;
import io.trino.spi.connector.CatalogHandle;
import io.trino.spi.connector.ConnectorMaterializedViewDefinition;
import io.trino.spi.connector.ConnectorMetadata;
import io.trino.spi.connector.ConnectorSession;
import io.trino.spi.connector.ConnectorViewDefinition;
Expand All @@ -44,6 +46,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
Expand All @@ -53,10 +56,15 @@
import static com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService;
import static io.airlift.json.JsonCodec.jsonCodec;
import static io.trino.metastore.TableInfo.ExtendedRelationType.TABLE;
import static io.trino.metastore.TableInfo.ExtendedRelationType.TRINO_MATERIALIZED_VIEW;
import static io.trino.metastore.TableInfo.ExtendedRelationType.TRINO_VIEW;
import static io.trino.plugin.hive.HiveErrorCode.HIVE_DATABASE_LOCATION_ERROR;
import static io.trino.plugin.iceberg.IcebergSchemaProperties.LOCATION_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.FORMAT_VERSION_PROPERTY;
import static io.trino.plugin.iceberg.IcebergUtil.quotedTableName;
import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.sql.planner.TestingPlannerContext.PLANNER_CONTEXT;
import static io.trino.testing.TestingConnectorSession.SESSION;
import static io.trino.testing.TestingNames.randomNameSuffix;
Expand Down Expand Up @@ -438,15 +446,79 @@ public void testListTables()
.commitTransaction();
closer.register(() -> catalog.dropTable(SESSION, table2));

ImmutableList.Builder<TableInfo> allTables = ImmutableList.<TableInfo>builder()
.add(new TableInfo(table1, TABLE))
.add(new TableInfo(table2, TABLE));

SchemaTableName view = new SchemaTableName(ns2, "view");
try {
catalog.createView(
SESSION,
view,
new ConnectorViewDefinition(
"SELECT name FROM local.tiny.nation",
Optional.empty(),
Optional.empty(),
ImmutableList.of(
new ConnectorViewDefinition.ViewColumn("name", VarcharType.createUnboundedVarcharType().getTypeId(), Optional.empty())),
Optional.empty(),
Optional.of(SESSION.getUser()),
false,
ImmutableList.of()),
false);
closer.register(() -> catalog.dropView(SESSION, view));
allTables.add(new TableInfo(view, getViewType()));
}
catch (TrinoException e) {
assertThat(e.getErrorCode()).isEqualTo(NOT_SUPPORTED.toErrorCode());
}

try {
SchemaTableName materializedView = new SchemaTableName(ns2, "mv");
catalog.createMaterializedView(
SESSION,
materializedView,
someMaterializedView(),
ImmutableMap.of(
FILE_FORMAT_PROPERTY, IcebergFileFormat.PARQUET,
FORMAT_VERSION_PROPERTY, 1),
false,
false);
closer.register(() -> catalog.dropMaterializedView(SESSION, materializedView));
allTables.add(new TableInfo(materializedView, TRINO_MATERIALIZED_VIEW));
}
catch (TrinoException e) {
assertThat(e.getErrorCode()).isEqualTo(NOT_SUPPORTED.toErrorCode());
}

createExternalIcebergTable(catalog, ns2, closer).ifPresent(table -> {
allTables.add(new TableInfo(table, TABLE));
});
createExternalNonIcebergTable(catalog, ns2, closer).ifPresent(table -> {
allTables.add(new TableInfo(table, TABLE));
});

// No namespace provided, all tables across all namespaces should be returned
assertThat(catalog.listTables(SESSION, Optional.empty())).containsAll(ImmutableList.of(new TableInfo(table1, TABLE), new TableInfo(table2, TABLE)));
assertThat(catalog.listTables(SESSION, Optional.empty())).containsAll(allTables.build());
// Namespace is provided and exists
assertThat(catalog.listTables(SESSION, Optional.of(ns1))).containsExactly(new TableInfo(table1, TABLE));
// Namespace is provided and does not exist
assertThat(catalog.listTables(SESSION, Optional.of("non_existing"))).isEmpty();
}
}

protected Optional<SchemaTableName> createExternalIcebergTable(TrinoCatalog catalog, String namespace, AutoCloseableCloser closer)
throws Exception
{
return Optional.empty();
}

protected Optional<SchemaTableName> createExternalNonIcebergTable(TrinoCatalog catalog, String namespace, AutoCloseableCloser closer)
throws Exception
{
return Optional.empty();
}

protected void assertViewDefinition(ConnectorViewDefinition actualView, ConnectorViewDefinition expectedView)
{
assertThat(actualView.getOriginalSql()).isEqualTo(expectedView.getOriginalSql());
Expand All @@ -460,7 +532,7 @@ protected void assertViewDefinition(ConnectorViewDefinition actualView, Connecto
assertThat(actualView.isRunAsInvoker()).isEqualTo(expectedView.isRunAsInvoker());
}

private String arbitraryTableLocation(TrinoCatalog catalog, ConnectorSession session, SchemaTableName schemaTableName)
protected String arbitraryTableLocation(TrinoCatalog catalog, ConnectorSession session, SchemaTableName schemaTableName)
throws Exception
{
try {
Expand All @@ -481,4 +553,18 @@ private void assertViewColumnDefinition(ConnectorViewDefinition.ViewColumn actua
assertThat(actualViewColumn.getName()).isEqualTo(expectedViewColumn.getName());
assertThat(actualViewColumn.getType()).isEqualTo(expectedViewColumn.getType());
}

private static ConnectorMaterializedViewDefinition someMaterializedView()
{
return new ConnectorMaterializedViewDefinition(
"select 1",
Optional.empty(),
Optional.empty(),
Optional.empty(),
ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("test", BIGINT.getTypeId(), Optional.empty())),
Optional.of(Duration.ZERO),
Optional.empty(),
Optional.of("owner"),
ImmutableList.of());
}
}
9 changes: 9 additions & 0 deletions ...test/java/io/trino/plugin/iceberg/catalog/file/TestTrinoHiveCatalogWithFileMetastore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import static io.trino.spi.type.IntegerType.INTEGER;
import static io.trino.testing.TestingConnectorSession.SESSION;
import static io.trino.testing.TestingNames.randomNameSuffix;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT;

Expand Down Expand Up @@ -154,4 +155,12 @@ private void testDropMaterializedView(boolean useUniqueTableLocations)
}
}
}

@Test
@Override
public void testListTables()
{
// the test actually works but when cleanup up the materialized view the error is thrown
assertThatThrownBy(super::testListTables).hasMessageMatching("Table 'ns2.*.mv' not found");
}
}
54 changes: 53 additions & 1 deletion .../test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.trino.hdfs.authentication.NoHdfsAuthentication;
import io.trino.hdfs.s3.HiveS3Config;
import io.trino.hdfs.s3.TrinoS3ConfigurationInitializer;
import io.trino.metastore.Table;
import io.trino.metastore.TableInfo;
import io.trino.plugin.base.util.AutoCloseableCloser;
import io.trino.plugin.hive.TrinoViewHiveMetastore;
Expand All @@ -51,6 +52,10 @@
import io.trino.spi.security.PrincipalType;
import io.trino.spi.security.TrinoPrincipal;
import io.trino.spi.type.TestingTypeManager;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.SortOrder;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand All @@ -65,6 +70,7 @@

import static com.google.common.base.Verify.verify;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static io.trino.metastore.PrincipalPrivileges.NO_PRIVILEGES;
import static io.trino.plugin.hive.TestingThriftHiveMetastoreBuilder.testingThriftHiveMetastoreBuilder;
import static io.trino.plugin.hive.containers.HiveHadoop.HIVE3_IMAGE;
import static io.trino.plugin.hive.metastore.cache.CachingHiveMetastore.createPerTransactionCache;
Expand All @@ -76,7 +82,10 @@
import static io.trino.testing.TestingNames.randomNameSuffix;
import static io.trino.testing.containers.Minio.MINIO_ACCESS_KEY;
import static io.trino.testing.containers.Minio.MINIO_SECRET_KEY;
import static java.util.Locale.ENGLISH;
import static java.util.concurrent.TimeUnit.MINUTES;
import static org.apache.iceberg.BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE;
import static org.apache.iceberg.BaseMetastoreTableOperations.TABLE_TYPE_PROP;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT;
Expand All @@ -92,6 +101,7 @@ public class TestTrinoHiveCatalogWithHiveMetastore
// Use MinIO for storage, since HDFS is hard to get working in a unit test
private HiveMinioDataLake dataLake;
private TrinoFileSystem fileSystem;
private CachingHiveMetastore metastore;
protected String bucketName;

HiveMinioDataLake hiveMinioDataLake()
Expand Down Expand Up @@ -138,7 +148,7 @@ protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations)
.setReadTimeout(new Duration(1, MINUTES)))
.metastoreClient(dataLake.getHiveMetastoreEndpoint())
.build(closer::register);
CachingHiveMetastore metastore = createPerTransactionCache(new BridgingHiveMetastore(thriftMetastore), 1000);
metastore = createPerTransactionCache(new BridgingHiveMetastore(thriftMetastore), 1000);
fileSystem = fileSystemFactory.create(SESSION);

return new TrinoHiveCatalog(
Expand Down Expand Up @@ -235,6 +245,48 @@ public void testCreateMaterializedView()
}
}

@Override
protected Optional<SchemaTableName> createExternalIcebergTable(TrinoCatalog catalog, String namespace, AutoCloseableCloser closer)
throws Exception
{
// simulate iceberg table created by spark with lowercase table type
return createTableWithTableType(catalog, namespace, closer, "lowercase_type", Optional.of(ICEBERG_TABLE_TYPE_VALUE.toLowerCase(ENGLISH)));
}

@Override
protected Optional<SchemaTableName> createExternalNonIcebergTable(TrinoCatalog catalog, String namespace, AutoCloseableCloser closer)
throws Exception
{
return createTableWithTableType(catalog, namespace, closer, "non_iceberg_table", Optional.empty());
}

private Optional<SchemaTableName> createTableWithTableType(TrinoCatalog catalog, String namespace, AutoCloseableCloser closer, String tableName, Optional<String> tableType)
throws Exception
{
SchemaTableName lowerCaseTableTypeTable = new SchemaTableName(namespace, tableName);
catalog.newCreateTableTransaction(
SESSION,
lowerCaseTableTypeTable,
new Schema(Types.NestedField.of(1, true, "col1", Types.LongType.get())),
PartitionSpec.unpartitioned(),
SortOrder.unsorted(),
arbitraryTableLocation(catalog, SESSION, lowerCaseTableTypeTable),
ImmutableMap.of())
.commitTransaction();

Table metastoreTable = metastore.getTable(namespace, tableName).get();

metastore.replaceTable(
namespace,
tableName,
Table.builder(metastoreTable)
.setParameter(TABLE_TYPE_PROP, tableType)
.build(),
NO_PRIVILEGES);
closer.register(() -> metastore.dropTable(namespace, tableName, true));
return Optional.of(lowerCaseTableTypeTable);
}

@Override
protected Map<String, Object> defaultNamespaceProperties(String namespaceName)
{
Expand Down

0 comments on commit 563ba35

Please sign in to comment.