diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java index 31ba12649d06..18f74440b87c 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java @@ -473,39 +473,38 @@ public LocatedTableHandle getTableHandle(ConnectorSession session, SchemaTableNa // Pretend the table does not exist to produce better error message in case of table redirects to Hive return null; } - SchemaTableName dataTableName = new SchemaTableName(tableName.getSchemaName(), tableName.getTableName()); - Optional table = metastore.getTable(dataTableName.getSchemaName(), dataTableName.getTableName()); + Optional table = metastore.getTable(tableName.getSchemaName(), tableName.getTableName()); if (table.isEmpty()) { return null; } boolean managed = table.get().managed(); String tableLocation = table.get().location(); - TableSnapshot tableSnapshot = getSnapshot(dataTableName, tableLocation, session); + TableSnapshot tableSnapshot = getSnapshot(tableName, tableLocation, session); MetadataEntry metadataEntry; try { metadataEntry = transactionLogAccess.getMetadataEntry(tableSnapshot, session); } catch (TrinoException e) { if (e.getErrorCode().equals(DELTA_LAKE_INVALID_SCHEMA.toErrorCode())) { - return new CorruptedDeltaLakeTableHandle(dataTableName, managed, tableLocation, e); + return new CorruptedDeltaLakeTableHandle(tableName, managed, tableLocation, e); } throw e; } ProtocolEntry protocolEntry = transactionLogAccess.getProtocolEntry(session, tableSnapshot); if (protocolEntry.getMinReaderVersion() > MAX_READER_VERSION) { - LOG.debug("Skip %s because the reader version is unsupported: %d", dataTableName, protocolEntry.getMinReaderVersion()); + LOG.debug("Skip %s because the reader version is unsupported: %d", tableName, protocolEntry.getMinReaderVersion()); return null; } Set unsupportedReaderFeatures = unsupportedReaderFeatures(protocolEntry.getReaderFeatures().orElse(ImmutableSet.of())); if (!unsupportedReaderFeatures.isEmpty()) { - LOG.debug("Skip %s because the table contains unsupported reader features: %s", dataTableName, unsupportedReaderFeatures); + LOG.debug("Skip %s because the table contains unsupported reader features: %s", tableName, unsupportedReaderFeatures); return null; } verifySupportedColumnMapping(getColumnMappingMode(metadataEntry)); return new DeltaLakeTableHandle( - dataTableName.getSchemaName(), - dataTableName.getTableName(), + tableName.getSchemaName(), + tableName.getTableName(), managed, tableLocation, metadataEntry, @@ -546,16 +545,12 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect // This method does not calculate column metadata for the projected columns checkArgument(tableHandle.getProjectedColumns().isEmpty(), "Unexpected projected columns"); MetadataEntry metadataEntry = tableHandle.getMetadataEntry(); - Map columnComments = getColumnComments(metadataEntry); - Map columnsNullability = getColumnsNullability(metadataEntry); - Map columnGenerations = getGeneratedColumnExpressions(metadataEntry); + List constraints = ImmutableList.builder() .addAll(getCheckConstraints(metadataEntry).values()) .addAll(getColumnInvariants(metadataEntry).values()) // The internal logic for column invariants in Delta Lake is same as check constraints .build(); - List columns = getColumns(metadataEntry).stream() - .map(column -> getColumnMetadata(column, columnComments.get(column.getBaseColumnName()), columnsNullability.getOrDefault(column.getBaseColumnName(), true), columnGenerations.get(column.getBaseColumnName()))) - .collect(toImmutableList()); + List columns = getTableColumnMetadata(metadataEntry); ImmutableMap.Builder properties = ImmutableMap.builder() .put(LOCATION_PROPERTY, tableHandle.getLocation()); @@ -592,6 +587,17 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect .collect(toImmutableList())); } + private List getTableColumnMetadata(MetadataEntry metadataEntry) + { + Map columnComments = getColumnComments(metadataEntry); + Map columnsNullability = getColumnsNullability(metadataEntry); + Map columnGenerations = getGeneratedColumnExpressions(metadataEntry); + List columns = getColumns(metadataEntry).stream() + .map(column -> getColumnMetadata(column, columnComments.get(column.getBaseColumnName()), columnsNullability.getOrDefault(column.getBaseColumnName(), true), columnGenerations.get(column.getBaseColumnName()))) + .collect(toImmutableList()); + return columns; + } + @Override public List listTables(ConnectorSession session, Optional schemaName) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java index e2bec0d34ec9..d628d4e82070 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java @@ -279,9 +279,9 @@ import static io.trino.plugin.hive.util.AcidTables.writeAcidVersionFile; import static io.trino.plugin.hive.util.HiveBucketing.getHiveBucketHandle; import static io.trino.plugin.hive.util.HiveBucketing.isSupportedBucketing; -import static io.trino.plugin.hive.util.HiveUtil.columnMetadataGetter; import static io.trino.plugin.hive.util.HiveUtil.getPartitionKeyColumnHandles; import static io.trino.plugin.hive.util.HiveUtil.getRegularColumnHandles; +import static io.trino.plugin.hive.util.HiveUtil.getTableColumnMetadata; import static io.trino.plugin.hive.util.HiveUtil.hiveColumnHandles; import static io.trino.plugin.hive.util.HiveUtil.isDeltaLakeTable; import static io.trino.plugin.hive.util.HiveUtil.isHiveSystemSchema; @@ -634,11 +634,7 @@ else if (isTrinoView || isTrinoMaterializedView) { throw new TableNotFoundException(tableName); } - Function metadataGetter = columnMetadataGetter(table); - ImmutableList.Builder columns = ImmutableList.builder(); - for (HiveColumnHandle columnHandle : hiveColumnHandles(table, typeManager, getTimestampPrecision(session))) { - columns.add(metadataGetter.apply(columnHandle)); - } + List columns = getTableColumnMetadata(session, table, typeManager); // External location property ImmutableMap.Builder properties = ImmutableMap.builder(); @@ -737,7 +733,7 @@ else if (isTrinoView || isTrinoMaterializedView) { // Partition Projection specific properties properties.putAll(partitionProjectionService.getPartitionProjectionTrinoTableProperties(table)); - return new ConnectorTableMetadata(tableName, columns.build(), properties.buildOrThrow(), comment); + return new ConnectorTableMetadata(tableName, columns, properties.buildOrThrow(), comment); } private static Optional getCsvSerdeProperty(Table table, String key) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java index d437438d43ee..88a08ec0c120 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java @@ -134,6 +134,7 @@ import java.util.concurrent.Future; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Stream; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; @@ -462,15 +463,7 @@ public Optional> getAllViews() private List getTableNames(String databaseName, Predicate filter) { try { - List tableNames = getPaginatedResults( - glueClient::getTables, - new GetTablesRequest() - .withDatabaseName(databaseName), - GetTablesRequest::setNextToken, - GetTablesResult::getNextToken, - stats.getGetTables()) - .map(GetTablesResult::getTableList) - .flatMap(List::stream) + List tableNames = getGlueTables(databaseName) .filter(filter) .map(com.amazonaws.services.glue.model.Table::getName) .collect(toImmutableList()); @@ -1164,6 +1157,19 @@ public void checkSupportsTransactions() throw new TrinoException(NOT_SUPPORTED, "Glue does not support ACID tables"); } + private Stream getGlueTables(String databaseName) + { + return getPaginatedResults( + glueClient::getTables, + new GetTablesRequest() + .withDatabaseName(databaseName), + GetTablesRequest::setNextToken, + GetTablesResult::getNextToken, + stats.getGetTables()) + .map(GetTablesResult::getTableList) + .flatMap(List::stream); + } + static class StatsRecordingAsyncHandler implements AsyncHandler { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java index e0789bc9bf38..a5aae9edb77d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java @@ -36,6 +36,7 @@ import io.trino.spi.ErrorCodeSupplier; import io.trino.spi.TrinoException; import io.trino.spi.connector.ColumnMetadata; +import io.trino.spi.connector.ConnectorSession; import io.trino.spi.predicate.NullableValue; import io.trino.spi.type.ArrayType; import io.trino.spi.type.CharType; @@ -93,6 +94,7 @@ import static io.trino.plugin.hive.HiveMetadata.SKIP_FOOTER_COUNT_KEY; import static io.trino.plugin.hive.HiveMetadata.SKIP_HEADER_COUNT_KEY; import static io.trino.plugin.hive.HivePartitionKey.HIVE_DEFAULT_DYNAMIC_PARTITION; +import static io.trino.plugin.hive.HiveSessionProperties.getTimestampPrecision; import static io.trino.plugin.hive.HiveTableProperties.ORC_BLOOM_FILTER_FPP; import static io.trino.plugin.hive.HiveType.toHiveTypes; import static io.trino.plugin.hive.metastore.SortingColumn.Order.ASCENDING; @@ -511,6 +513,13 @@ private static Slice charPartitionKey(String value, String name, Type columnType return partitionKey; } + public static List getTableColumnMetadata(ConnectorSession session, Table table, TypeManager typeManager) + { + return hiveColumnHandles(table, typeManager, getTimestampPrecision(session)).stream() + .map(columnMetadataGetter(table)) + .collect(toImmutableList()); + } + public static List hiveColumnHandles(Table table, TypeManager typeManager, HiveTimestampPrecision timestampPrecision) { ImmutableList.Builder columns = ImmutableList.builder(); diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHiveLocal.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHiveLocal.java index f97a28461b84..a5835891020d 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHiveLocal.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHiveLocal.java @@ -18,6 +18,7 @@ import com.google.common.io.RecursiveDeleteOption; import com.google.common.reflect.ClassPath; import io.airlift.log.Logger; +import io.trino.filesystem.Location; import io.trino.plugin.hive.metastore.Column; import io.trino.plugin.hive.metastore.Database; import io.trino.plugin.hive.metastore.HiveMetastore; @@ -47,6 +48,7 @@ import java.net.URI; import java.nio.file.Files; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.OptionalInt; @@ -54,10 +56,15 @@ import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; import static io.trino.plugin.hive.HiveMetadata.PRESTO_QUERY_ID_NAME; import static io.trino.plugin.hive.HiveMetadata.PRESTO_VERSION_NAME; +import static io.trino.plugin.hive.HiveMetadata.TABLE_COMMENT; import static io.trino.plugin.hive.HiveStorageFormat.ORC; +import static io.trino.plugin.hive.HiveStorageFormat.TEXTFILE; import static io.trino.plugin.hive.HiveTestUtils.HDFS_ENVIRONMENT; import static io.trino.plugin.hive.HiveType.HIVE_INT; import static io.trino.plugin.hive.HiveType.HIVE_STRING; +import static io.trino.plugin.hive.TableType.MANAGED_TABLE; +import static io.trino.plugin.hive.metastore.PrincipalPrivileges.NO_PRIVILEGES; +import static io.trino.plugin.hive.metastore.StorageFormat.fromHiveStorageFormat; import static io.trino.plugin.hive.util.HiveBucketing.BucketingVersion.BUCKETING_V1; import static io.trino.plugin.hive.util.HiveUtil.SPARK_TABLE_PROVIDER_KEY; import static java.nio.file.Files.copy; @@ -105,6 +112,62 @@ public void initialize() .setRcfileTimeZone("America/Los_Angeles"); setup(testDbName, hiveConfig, metastore, HDFS_ENVIRONMENT); + + createTestTables(); + } + + protected void createTestTables() + throws Exception + { + Location location = Location.of((metastoreClient.getDatabase(database).orElseThrow() + .getLocation().orElseThrow())); + + createTestTable( + // Matches create-test.sql » trino_test_partition_format + Table.builder() + .setDatabaseName(database) + .setTableName(tablePartitionFormat.getTableName()) + .setTableType(MANAGED_TABLE.name()) + .setOwner(Optional.empty()) + .setDataColumns(List.of( + new Column("t_string", HiveType.HIVE_STRING, Optional.empty(), Map.of()), + new Column("t_tinyint", HiveType.HIVE_BYTE, Optional.empty(), Map.of()), + new Column("t_smallint", HiveType.HIVE_SHORT, Optional.empty(), Map.of()), + new Column("t_int", HiveType.HIVE_INT, Optional.empty(), Map.of()), + new Column("t_bigint", HiveType.HIVE_LONG, Optional.empty(), Map.of()), + new Column("t_float", HiveType.HIVE_FLOAT, Optional.empty(), Map.of()), + new Column("t_boolean", HiveType.HIVE_BOOLEAN, Optional.empty(), Map.of()))) + .setPartitionColumns(List.of( + new Column("ds", HiveType.HIVE_STRING, Optional.empty(), Map.of()), + new Column("file_format", HiveType.HIVE_STRING, Optional.empty(), Map.of()), + new Column("dummy", HiveType.HIVE_INT, Optional.empty(), Map.of()))) + .setParameter(TABLE_COMMENT, "Presto test data") + .withStorage(storage -> storage + .setStorageFormat(fromHiveStorageFormat(new HiveConfig().getHiveStorageFormat())) + .setLocation(Optional.of(location.appendPath(tablePartitionFormat.getTableName()).toString()))) + .build()); + + createTestTable( + // Matches create-test.sql » trino_test_partition_format + Table.builder() + .setDatabaseName(database) + .setTableName(tableUnpartitioned.getTableName()) + .setTableType(MANAGED_TABLE.name()) + .setOwner(Optional.empty()) + .setDataColumns(List.of( + new Column("t_string", HiveType.HIVE_STRING, Optional.empty(), Map.of()), + new Column("t_tinyint", HiveType.HIVE_BYTE, Optional.empty(), Map.of()))) + .setParameter(TABLE_COMMENT, "Presto test data") + .withStorage(storage -> storage + .setStorageFormat(fromHiveStorageFormat(TEXTFILE)) + .setLocation(Optional.of(location.appendPath(tableUnpartitioned.getTableName()).toString()))) + .build()); + } + + protected void createTestTable(Table table) + throws Exception + { + metastoreClient.createTable(table, NO_PRIVILEGES); } @AfterClass(alwaysRun = true) @@ -112,7 +175,10 @@ public void cleanup() throws IOException { try { - getMetastoreClient().dropDatabase(testDbName, true); + for (String tableName : metastoreClient.getAllTables(database)) { + metastoreClient.dropTable(database, tableName, true); + } + metastoreClient.dropDatabase(testDbName, true); } finally { deleteRecursively(tempDir.toPath(), ALLOW_INSECURE); @@ -128,12 +194,6 @@ protected ConnectorTableHandle getTableHandle(ConnectorMetadata metadata, Schema throw new SkipException("tests using existing tables are not supported"); } - @Override - public void testGetAllTableNames() - { - throw new SkipException("Test disabled for this subclass"); - } - @Override public void testGetAllTableColumns() { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveInMemoryMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveInMemoryMetastore.java index 7de52af417b9..cb8b161be152 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveInMemoryMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveInMemoryMetastore.java @@ -14,6 +14,7 @@ package io.trino.plugin.hive; import io.trino.plugin.hive.metastore.HiveMetastore; +import io.trino.plugin.hive.metastore.Table; import io.trino.plugin.hive.metastore.thrift.BridgingHiveMetastore; import io.trino.plugin.hive.metastore.thrift.InMemoryThriftMetastore; import io.trino.plugin.hive.metastore.thrift.ThriftMetastoreConfig; @@ -21,7 +22,9 @@ import org.testng.annotations.Test; import java.io.File; +import java.net.URI; +import static java.nio.file.Files.createDirectories; import static org.assertj.core.api.Assertions.assertThatThrownBy; // staging directory is shared mutable state @@ -38,6 +41,14 @@ protected HiveMetastore createMetastore(File tempDir) return new BridgingHiveMetastore(hiveMetastore); } + @Override + protected void createTestTable(Table table) + throws Exception + { + createDirectories(new File(URI.create(table.getStorage().getLocation())).toPath()); + super.createTestTable(table); + } + @Test public void forceTestNgToRespectSingleThreaded() { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/CountingAccessHiveMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/CountingAccessHiveMetastore.java index 5f4a7cd914d9..921336939a1b 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/CountingAccessHiveMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/CountingAccessHiveMetastore.java @@ -49,7 +49,7 @@ public enum Method GET_TABLE, GET_ALL_TABLES, GET_ALL_TABLES_FROM_DATABASE, - GET_TABLE_WITH_PARAMETER, + GET_TABLES_WITH_PARAMETER, GET_TABLE_STATISTICS, GET_ALL_VIEWS, GET_ALL_VIEWS_FROM_DATABASE, @@ -113,7 +113,7 @@ public Optional getDatabase(String databaseName) @Override public List getTablesWithParameter(String databaseName, String parameterKey, String parameterValue) { - methodInvocations.add(Method.GET_TABLE_WITH_PARAMETER); + methodInvocations.add(Method.GET_TABLES_WITH_PARAMETER); return delegate.getTablesWithParameter(databaseName, parameterKey, parameterValue); } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/thrift/InMemoryThriftMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/thrift/InMemoryThriftMetastore.java index cfb678985521..adea5b768c0e 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/thrift/InMemoryThriftMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/thrift/InMemoryThriftMetastore.java @@ -184,7 +184,7 @@ public synchronized void createTable(Table table) } else { File directory = new File(new Path(table.getSd().getLocation()).toUri()); - checkArgument(directory.exists(), "Table directory does not exist"); + checkArgument(directory.exists(), "Table directory [%s] does not exist", directory); if (tableType == MANAGED_TABLE) { checkArgument(isParentDir(directory, baseDirectory), "Table directory must be inside of the metastore base directory"); } @@ -202,7 +202,7 @@ public synchronized void createTable(Table table) } PrincipalPrivilegeSet privileges = table.getPrivileges(); - if (privileges != null) { + if (privileges != null && (!privileges.getUserPrivileges().isEmpty() || !privileges.getGroupPrivileges().isEmpty() || !privileges.getRolePrivileges().isEmpty())) { throw new UnsupportedOperationException(); } } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMetastoreAccessOperations.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMetastoreAccessOperations.java index 0706f08e4e7b..394d8a5a8c22 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMetastoreAccessOperations.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMetastoreAccessOperations.java @@ -34,7 +34,7 @@ import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.GET_ALL_TABLES_FROM_DATABASE; import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.GET_DATABASE; import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.GET_TABLE; -import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.GET_TABLE_WITH_PARAMETER; +import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.GET_TABLES_WITH_PARAMETER; import static io.trino.plugin.hive.metastore.CountingAccessHiveMetastore.Method.REPLACE_TABLE; import static io.trino.plugin.hive.metastore.file.TestingFileHiveMetastore.createTestingFileHiveMetastore; import static io.trino.plugin.iceberg.IcebergSessionProperties.COLLECT_EXTENDED_STATISTICS_ON_WRITE; @@ -348,7 +348,7 @@ public void testInformationSchemaColumns(int tables) ImmutableMultiset.builder() .add(GET_ALL_TABLES_FROM_DATABASE) .addCopies(GET_TABLE, tables * 2) - .addCopies(GET_TABLE_WITH_PARAMETER, 2) + .addCopies(GET_TABLES_WITH_PARAMETER, 2) .build()); for (int i = 0; i < tables; i++) { @@ -380,7 +380,7 @@ public void testSystemMetadataTableComments(int tables) ImmutableMultiset.builder() .add(GET_ALL_TABLES_FROM_DATABASE) .addCopies(GET_TABLE, tables * 2) - .addCopies(GET_TABLE_WITH_PARAMETER, 2) + .addCopies(GET_TABLES_WITH_PARAMETER, 2) .build()); // Pointed lookup