diff --git a/.mvn/modernizer/violations.xml b/.mvn/modernizer/violations.xml index 6b4ad47ca408..737bb7d41a22 100644 --- a/.mvn/modernizer/violations.xml +++ b/.mvn/modernizer/violations.xml @@ -152,6 +152,24 @@ Table type is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getTableType + + com/amazonaws/services/glue/model/Table.getParameters:()Ljava/util/Map; + 1.1 + Table parameters map is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getTableParameters + + + + com/amazonaws/services/glue/model/Partition.getParameters:()Ljava/util/Map; + 1.1 + Partition parameters map is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getPartitionParameters + + + + com/amazonaws/services/glue/model/SerDeInfo.getParameters:()Ljava/util/Map; + 1.1 + SerDeInfo parameters map is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getSerDeInfoParameters + + org/apache/hadoop/mapred/JobConf."<init>":()V 1.1 diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java index 643397f41494..22efedcad546 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java @@ -150,15 +150,22 @@ public static boolean isHiveOrPrestoView(String tableType) return tableType.equals(VIRTUAL_VIEW.name()); } + public static boolean isTrinoMaterializedView(Table table) + { + return isTrinoMaterializedView(table.getTableType(), table.getParameters()); + } + public static boolean isTrinoMaterializedView(String tableType, Map tableParameters) { + // TODO isHiveOrPrestoView should not return true for materialized views return isHiveOrPrestoView(tableType) && isPrestoView(tableParameters) && tableParameters.get(TABLE_COMMENT).equalsIgnoreCase(ICEBERG_MATERIALIZED_VIEW_COMMENT); } public static boolean canDecodeView(Table table) { // we can decode Hive or Presto view - return table.getTableType().equals(VIRTUAL_VIEW.name()); + return table.getTableType().equals(VIRTUAL_VIEW.name()) && + !isTrinoMaterializedView(table.getTableType(), table.getParameters()); } public static String encodeViewData(ConnectorViewDefinition definition) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/DefaultGlueMetastoreTableFilterProvider.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/DefaultGlueMetastoreTableFilterProvider.java index 63ef87298ec3..b80a76c23337 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/DefaultGlueMetastoreTableFilterProvider.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/DefaultGlueMetastoreTableFilterProvider.java @@ -22,7 +22,7 @@ import java.util.Map; import java.util.function.Predicate; -import static com.google.common.base.MoreObjects.firstNonNull; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.hive.util.HiveUtil.DELTA_LAKE_PROVIDER; import static io.trino.plugin.hive.util.HiveUtil.SPARK_TABLE_PROVIDER_KEY; import static java.util.function.Predicate.not; @@ -49,7 +49,7 @@ public Predicate get() public static boolean isDeltaLakeTable(Table table) { - Map parameters = firstNonNull(table.getParameters(), Map.of()); + Map parameters = getTableParameters(table); return parameters.getOrDefault(SPARK_TABLE_PROVIDER_KEY, "").equalsIgnoreCase(DELTA_LAKE_PROVIDER); } } 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 55b0eeea8a52..84927baacecb 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 @@ -132,7 +132,6 @@ import java.util.function.Function; import java.util.function.Predicate; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; import static com.google.common.collect.Comparators.lexicographical; @@ -149,6 +148,7 @@ import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults; import static io.trino.plugin.hive.metastore.glue.GlueClientUtil.createAsyncGlueClient; import static io.trino.plugin.hive.metastore.glue.converter.GlueInputConverter.convertPartition; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableTypeNullable; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.mappedCopy; import static io.trino.plugin.hive.metastore.thrift.ThriftMetastoreUtil.getHiveBasicStatistics; @@ -442,7 +442,7 @@ public List getAllTables(String databaseName) @Override public synchronized List getTablesWithParameter(String databaseName, String parameterKey, String parameterValue) { - return getAllViews(databaseName, table -> parameterValue.equals(firstNonNull(table.getParameters(), ImmutableMap.of()).get(parameterKey))); + return getAllViews(databaseName, table -> parameterValue.equals(getTableParameters(table).get(parameterKey))); } @Override @@ -680,7 +680,7 @@ private TableInput convertGlueTableToTableInput(com.amazonaws.services.glue.mode .withViewExpandedText(glueTable.getViewExpandedText()) .withTableType(getTableTypeNullable(glueTable)) .withTargetTable(glueTable.getTargetTable()) - .withParameters(glueTable.getParameters()); + .withParameters(getTableParameters(glueTable)); } @Override diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java index e6c2307846a7..d880b9483335 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java @@ -51,6 +51,7 @@ import static io.trino.plugin.hive.HiveErrorCode.HIVE_UNSUPPORTED_FORMAT; import static io.trino.plugin.hive.HiveType.HIVE_INT; import static io.trino.plugin.hive.TableType.EXTERNAL_TABLE; +import static io.trino.plugin.hive.ViewReaderUtil.isTrinoMaterializedView; import static io.trino.plugin.hive.metastore.util.Memoizers.memoizeLast; import static io.trino.plugin.hive.util.HiveUtil.isDeltaLakeTable; import static io.trino.plugin.hive.util.HiveUtil.isIcebergTable; @@ -76,6 +77,24 @@ public static String getTableTypeNullable(com.amazonaws.services.glue.model.Tabl return glueTable.getTableType(); } + @SuppressModernizer // Usage of `Table.getParameters` is not allowed. Only this method can call that. + public static Map getTableParameters(com.amazonaws.services.glue.model.Table glueTable) + { + return firstNonNull(glueTable.getParameters(), ImmutableMap.of()); + } + + @SuppressModernizer // Usage of `Partition.getParameters` is not allowed. Only this method can call that. + public static Map getPartitionParameters(com.amazonaws.services.glue.model.Partition gluePartition) + { + return firstNonNull(gluePartition.getParameters(), ImmutableMap.of()); + } + + @SuppressModernizer // Usage of `SerDeInfo.getParameters` is not allowed. Only this method can call that. + public static Map getSerDeInfoParameters(com.amazonaws.services.glue.model.SerDeInfo glueSerDeInfo) + { + return firstNonNull(glueSerDeInfo.getParameters(), ImmutableMap.of()); + } + public static Database convertDatabase(com.amazonaws.services.glue.model.Database glueDb) { return Database.builder() @@ -95,21 +114,25 @@ public static Table convertTable(com.amazonaws.services.glue.model.Table glueTab { SchemaTableName table = new SchemaTableName(dbName, glueTable.getName()); - Map tableParameters = convertParameters(glueTable.getParameters()); + String tableType = getTableType(glueTable); + Map tableParameters = ImmutableMap.copyOf(getTableParameters(glueTable)); Table.Builder tableBuilder = Table.builder() .setDatabaseName(table.getSchemaName()) .setTableName(table.getTableName()) .setOwner(Optional.ofNullable(glueTable.getOwner())) - .setTableType(getTableType(glueTable)) + .setTableType(tableType) .setParameters(tableParameters) .setViewOriginalText(Optional.ofNullable(glueTable.getViewOriginalText())) .setViewExpandedText(Optional.ofNullable(glueTable.getViewExpandedText())); StorageDescriptor sd = glueTable.getStorageDescriptor(); - if (isIcebergTable(tableParameters) || (sd == null && isDeltaLakeTable(tableParameters))) { + if (isIcebergTable(tableParameters) || + (sd == null && isDeltaLakeTable(tableParameters)) || + (sd == null && isTrinoMaterializedView(tableType, tableParameters))) { // Iceberg tables do not need to read the StorageDescriptor field, but we still need to return dummy properties for compatibility // Delta Lake tables only need to provide a dummy properties if a StorageDescriptor was not explicitly configured. + // Materialized views do not need to read the StorageDescriptor, but we still need to return dummy properties for compatibility tableBuilder.setDataColumns(ImmutableList.of(new Column("dummy", HIVE_INT, Optional.empty()))); tableBuilder.getStorageBuilder().setStorageFormat(StorageFormat.fromHiveStorageFormat(HiveStorageFormat.PARQUET)); } @@ -157,17 +180,9 @@ private static List convertColumns(SchemaTableName table, List convertColumn(table, glueColumn, serde)); } - private static Map convertParameters(Map parameters) - { - if (parameters == null || parameters.isEmpty()) { - return ImmutableMap.of(); - } - return ImmutableMap.copyOf(parameters); - } - private static Function, Map> parametersConverter() { - return memoizeLast(GlueToTrinoConverter::convertParameters); + return memoizeLast(ImmutableMap::copyOf); } private static boolean isNullOrEmpty(List list) @@ -190,7 +205,7 @@ public GluePartitionConverter(Table table) requireNonNull(table, "table is null"); this.databaseName = requireNonNull(table.getDatabaseName(), "databaseName is null"); this.tableName = requireNonNull(table.getTableName(), "tableName is null"); - this.tableParameters = convertParameters(table.getParameters()); + this.tableParameters = table.getParameters(); this.columnsConverter = memoizeLast(glueColumns -> convertColumns( table.getSchemaTableName(), glueColumns, @@ -214,7 +229,7 @@ public Partition apply(com.amazonaws.services.glue.model.Partition gluePartition .setTableName(tableName) .setValues(gluePartition.getValues()) // No memoization benefit .setColumns(columnsConverter.apply(sd.getColumns())) - .setParameters(parametersConverter.apply(gluePartition.getParameters())); + .setParameters(parametersConverter.apply(getPartitionParameters(gluePartition))); storageConverter.setStorageBuilder(sd, partitionBuilder.getStorageBuilder(), tableParameters); @@ -239,7 +254,7 @@ public void setStorageBuilder(StorageDescriptor sd, Storage.Builder storageBuild .setLocation(nullToEmpty(sd.getLocation())) .setBucketProperty(convertToBucketProperty(tableParameters, sd)) .setSkewed(sd.getSkewedInfo() != null && !isNullOrEmpty(sd.getSkewedInfo().getSkewedColumnNames())) - .setSerdeParameters(serdeParametersConverter.apply(serdeInfo.getParameters())) + .setSerdeParameters(serdeParametersConverter.apply(getSerDeInfoParameters(serdeInfo))) .build(); } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java index 93f2ac929eaf..5f391aa7c7ca 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java @@ -41,6 +41,9 @@ import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestPartition; import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestStorageDescriptor; import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestTable; +import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestTrinoMaterializedView; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getPartitionParameters; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableTypeNullable; import static io.trino.plugin.hive.util.HiveUtil.DELTA_LAKE_PROVIDER; import static io.trino.plugin.hive.util.HiveUtil.ICEBERG_TABLE_TYPE_NAME; @@ -96,7 +99,7 @@ public void testConvertTable() assertEquals(trinoTable.getDatabaseName(), testDatabase.getName()); assertEquals(trinoTable.getTableType(), getTableTypeNullable(testTable)); assertEquals(trinoTable.getOwner().orElse(null), testTable.getOwner()); - assertEquals(trinoTable.getParameters(), testTable.getParameters()); + assertEquals(trinoTable.getParameters(), getTableParameters(testTable)); assertColumnList(trinoTable.getDataColumns(), testTable.getStorageDescriptor().getColumns()); assertColumnList(trinoTable.getPartitionColumns(), testTable.getPartitionKeys()); assertStorage(trinoTable.getStorage(), testTable.getStorageDescriptor()); @@ -117,7 +120,7 @@ public void testConvertTableWithOpenCSVSerDe() assertEquals(trinoTable.getDatabaseName(), testDatabase.getName()); assertEquals(trinoTable.getTableType(), getTableTypeNullable(glueTable)); assertEquals(trinoTable.getOwner().orElse(null), glueTable.getOwner()); - assertEquals(trinoTable.getParameters(), glueTable.getParameters()); + assertEquals(trinoTable.getParameters(), getTableParameters(glueTable)); assertEquals(trinoTable.getDataColumns().size(), 1); assertEquals(trinoTable.getDataColumns().get(0).getType(), HIVE_STRING); @@ -162,7 +165,7 @@ public void testConvertPartition() assertColumnList(trinoPartition.getColumns(), testPartition.getStorageDescriptor().getColumns()); assertEquals(trinoPartition.getValues(), testPartition.getValues()); assertStorage(trinoPartition.getStorage(), testPartition.getStorageDescriptor()); - assertEquals(trinoPartition.getParameters(), testPartition.getParameters()); + assertEquals(trinoPartition.getParameters(), getPartitionParameters(testPartition)); } @Test @@ -259,6 +262,15 @@ public void testDeltaTableNonNullStorageDescriptor() .collect(toImmutableSet())); } + @Test + public void testIcebergMaterializedViewNullStorageDescriptor() + { + Table testMaterializedView = getGlueTestTrinoMaterializedView(testDatabase.getName()); + assertNull(testMaterializedView.getStorageDescriptor()); + io.trino.plugin.hive.metastore.Table trinoTable = GlueToTrinoConverter.convertTable(testMaterializedView, testDatabase.getName()); + assertEquals(trinoTable.getDataColumns().size(), 1); + } + @Test public void testPartitionNullParameters() { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java index 9a05b1db007f..cc0dc7034981 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java @@ -80,6 +80,7 @@ import java.util.OptionalLong; import java.util.Set; import java.util.concurrent.Executor; +import java.util.function.Supplier; import static com.google.common.collect.ImmutableList.toImmutableList; import static io.airlift.concurrent.MoreFutures.getFutureValue; @@ -89,9 +90,13 @@ import static io.trino.plugin.hive.HiveColumnStatisticType.MIN_VALUE; import static io.trino.plugin.hive.HiveColumnStatisticType.NUMBER_OF_DISTINCT_VALUES; import static io.trino.plugin.hive.HiveColumnStatisticType.NUMBER_OF_NON_NULL_VALUES; +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.ViewReaderUtil.ICEBERG_MATERIALIZED_VIEW_COMMENT; +import static io.trino.plugin.hive.ViewReaderUtil.PRESTO_VIEW_FLAG; +import static io.trino.plugin.hive.ViewReaderUtil.isTrinoMaterializedView; import static io.trino.plugin.hive.acid.AcidTransaction.NO_ACID_TRANSACTION; import static io.trino.plugin.hive.metastore.HiveColumnStatistics.createIntegerColumnStatistics; import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults; @@ -118,6 +123,7 @@ import static java.util.concurrent.TimeUnit.DAYS; import static org.apache.hadoop.hive.common.FileUtils.makePartName; import static org.apache.hadoop.hive.metastore.TableType.EXTERNAL_TABLE; +import static org.apache.hadoop.hive.metastore.TableType.VIRTUAL_VIEW; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.testng.Assert.assertEquals; @@ -1293,17 +1299,21 @@ public void testInvalidColumnStatisticsMetadata() } @Test - public void testTableWithoutStorageDescriptor() + public void testGlueObjectsWithoutStorageDescriptor() { - // StorageDescriptor is an Optional field for Glue tables. Iceberg and Delta Lake tables may not have it set. + // StorageDescriptor is an Optional field for Glue tables. SchemaTableName table = temporaryTable("test_missing_storage_descriptor"); DeleteTableRequest deleteTableRequest = new DeleteTableRequest() .withDatabaseName(table.getSchemaName()) .withName(table.getTableName()); + try { - TableInput tableInput = new TableInput() + Supplier resetTableInput = () -> new TableInput() + .withStorageDescriptor(null) .withName(table.getTableName()) .withTableType(EXTERNAL_TABLE.name()); + + TableInput tableInput = resetTableInput.get(); glueClient.createTable(new CreateTableRequest() .withDatabaseName(database) .withTableInput(tableInput)); @@ -1313,7 +1323,7 @@ public void testTableWithoutStorageDescriptor() glueClient.deleteTable(deleteTableRequest); // Iceberg table - tableInput = tableInput.withParameters(ImmutableMap.of(ICEBERG_TABLE_TYPE_NAME, ICEBERG_TABLE_TYPE_VALUE)); + tableInput = resetTableInput.get().withParameters(ImmutableMap.of(ICEBERG_TABLE_TYPE_NAME, ICEBERG_TABLE_TYPE_VALUE)); glueClient.createTable(new CreateTableRequest() .withDatabaseName(database) .withTableInput(tableInput)); @@ -1321,11 +1331,30 @@ public void testTableWithoutStorageDescriptor() glueClient.deleteTable(deleteTableRequest); // Delta Lake table - tableInput = tableInput.withParameters(ImmutableMap.of(SPARK_TABLE_PROVIDER_KEY, DELTA_LAKE_PROVIDER)); + tableInput = resetTableInput.get().withParameters(ImmutableMap.of(SPARK_TABLE_PROVIDER_KEY, DELTA_LAKE_PROVIDER)); glueClient.createTable(new CreateTableRequest() .withDatabaseName(database) .withTableInput(tableInput)); assertTrue(isDeltaLakeTable(metastore.getTable(table.getSchemaName(), table.getTableName()).orElseThrow())); + glueClient.deleteTable(deleteTableRequest); + + // Iceberg materialized view + tableInput = resetTableInput.get().withTableType(VIRTUAL_VIEW.name()) + .withViewOriginalText("/* Presto Materialized View: eyJvcmlnaW5hbFNxbCI6IlNFTEVDVCAxIiwiY29sdW1ucyI6W3sibmFtZSI6ImEiLCJ0eXBlIjoiaW50ZWdlciJ9XX0= */") + .withViewExpandedText(ICEBERG_MATERIALIZED_VIEW_COMMENT) + .withParameters(ImmutableMap.of( + PRESTO_VIEW_FLAG, "true", + TABLE_COMMENT, ICEBERG_MATERIALIZED_VIEW_COMMENT)); + glueClient.createTable(new CreateTableRequest() + .withDatabaseName(database) + .withTableInput(tableInput)); + assertTrue(isTrinoMaterializedView(metastore.getTable(table.getSchemaName(), table.getTableName()).orElseThrow())); + try (Transaction transaction = newTransaction()) { + ConnectorSession session = newSession(); + ConnectorMetadata metadata = transaction.getMetadata(); + // Not a view + assertThat(metadata.getView(session, table)).isEmpty(); + } } finally { // Table cannot be dropped through HiveMetastore since a TableHandle cannot be created diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingMetastoreObjects.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingMetastoreObjects.java index aa18a77e816b..0594cbf8814b 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingMetastoreObjects.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingMetastoreObjects.java @@ -32,6 +32,9 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.function.Consumer; +import static io.trino.plugin.hive.HiveMetadata.TABLE_COMMENT; +import static io.trino.plugin.hive.ViewReaderUtil.ICEBERG_MATERIALIZED_VIEW_COMMENT; +import static io.trino.plugin.hive.ViewReaderUtil.PRESTO_VIEW_FLAG; import static java.lang.String.format; public final class TestingMetastoreObjects @@ -63,6 +66,20 @@ public static Table getGlueTestTable(String dbName) .withViewExpandedText("expandedText"); } + public static Table getGlueTestTrinoMaterializedView(String dbName) + { + return new Table() + .withDatabaseName(dbName) + .withName("test-mv" + generateRandom()) + .withOwner("owner") + .withParameters(ImmutableMap.of(PRESTO_VIEW_FLAG, "true", TABLE_COMMENT, ICEBERG_MATERIALIZED_VIEW_COMMENT)) + .withPartitionKeys() + .withStorageDescriptor(null) + .withTableType(TableType.VIRTUAL_VIEW.name()) + .withViewOriginalText("/* %s: base64encodedquery */".formatted(ICEBERG_MATERIALIZED_VIEW_COMMENT)) + .withViewExpandedText(ICEBERG_MATERIALIZED_VIEW_COMMENT); + } + public static Column getGlueTestColumn() { return getGlueTestColumn("string"); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueIcebergTableOperations.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueIcebergTableOperations.java index 3201a9963c87..254442cbb9b6 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueIcebergTableOperations.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/GlueIcebergTableOperations.java @@ -41,10 +41,10 @@ import java.util.Map; import java.util.Optional; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Verify.verify; import static io.trino.plugin.hive.ViewReaderUtil.isHiveOrPrestoView; import static io.trino.plugin.hive.ViewReaderUtil.isPrestoView; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableType; import static io.trino.plugin.hive.util.HiveUtil.isIcebergTable; import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_INVALID_METADATA; @@ -84,7 +84,7 @@ protected String getRefreshedLocation(boolean invalidateCaches) Table table = getTable(); glueVersionId = table.getVersionId(); - Map parameters = firstNonNull(table.getParameters(), ImmutableMap.of()); + Map parameters = getTableParameters(table); if (isPrestoView(parameters) && isHiveOrPrestoView(getTableType(table))) { // this is a Presto Hive view, hence not a table throw new TableNotFoundException(getSchemaTableName()); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index 0f601eafa53c..e9c6db51d932 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -77,7 +77,6 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; @@ -91,6 +90,7 @@ import static io.trino.plugin.hive.ViewReaderUtil.isPrestoView; import static io.trino.plugin.hive.ViewReaderUtil.isTrinoMaterializedView; import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableType; import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableTypeNullable; import static io.trino.plugin.hive.util.HiveUtil.isHiveSystemSchema; @@ -380,7 +380,7 @@ public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) public void dropCorruptedTable(ConnectorSession session, SchemaTableName schemaTableName) { com.amazonaws.services.glue.model.Table table = dropTableFromMetastore(session, schemaTableName); - String metadataLocation = table.getParameters().get(METADATA_LOCATION_PROP); + String metadataLocation = getTableParameters(table).get(METADATA_LOCATION_PROP); if (metadataLocation == null) { throw new TrinoException(ICEBERG_INVALID_METADATA, format("Table %s is missing [%s] property", schemaTableName, METADATA_LOCATION_PROP)); } @@ -427,7 +427,7 @@ private com.amazonaws.services.glue.model.Table dropTableFromMetastore(Connector { com.amazonaws.services.glue.model.Table table = getTable(session, schemaTableName) .orElseThrow(() -> new TableNotFoundException(schemaTableName)); - if (!isIcebergTable(firstNonNull(table.getParameters(), ImmutableMap.of()))) { + if (!isIcebergTable(getTableParameters(table))) { throw new UnknownTableTypeException(schemaTableName); } @@ -447,7 +447,7 @@ public void renameTable(ConnectorSession session, SchemaTableName from, SchemaTa try { com.amazonaws.services.glue.model.Table table = getTable(session, from) .orElseThrow(() -> new TableNotFoundException(from)); - TableInput tableInput = getTableInput(to.getTableName(), Optional.ofNullable(table.getOwner()), table.getParameters()); + TableInput tableInput = getTableInput(to.getTableName(), Optional.ofNullable(table.getOwner()), getTableParameters(table)); CreateTableRequest createTableRequest = new CreateTableRequest() .withDatabaseName(to.getSchemaName()) .withTableInput(tableInput); @@ -479,7 +479,7 @@ private Optional getTable(ConnectorSess .withName(schemaTableName.getTableName())) .getTable()); - Map parameters = firstNonNull(table.getParameters(), ImmutableMap.of()); + Map parameters = getTableParameters(table); if (isIcebergTable(parameters) && !tableMetadataCache.containsKey(schemaTableName)) { if (viewCache.containsKey(schemaTableName) || materializedViewCache.containsKey(schemaTableName)) { throw new TrinoException(GENERIC_INTERNAL_ERROR, "Glue table cache inconsistency. Table cannot also be a view/materialized view"); @@ -611,7 +611,7 @@ private void doCreateView(ConnectorSession session, SchemaTableName schemaViewNa { Optional existing = getTable(session, schemaViewName); if (existing.isPresent()) { - if (!replace || !isPrestoView(firstNonNull(existing.get().getParameters(), ImmutableMap.of()))) { + if (!replace || !isPrestoView(getTableParameters(existing.get()))) { // TODO: ViewAlreadyExists is misleading if the name is used by a table https://github.com/trinodb/trino/issues/10037 throw new ViewAlreadyExistsException(schemaViewName); } @@ -707,7 +707,7 @@ public List listViews(ConnectorSession session, Optional isPrestoView(firstNonNull(table.getParameters(), ImmutableMap.of()))) + .filter(table -> isPrestoView(getTableParameters(table))) .map(table -> new SchemaTableName(glueNamespace, table.getName())) .collect(toImmutableList())); } @@ -744,7 +744,7 @@ public Optional getView(ConnectorSession session, Schem viewName, Optional.ofNullable(viewDefinition.getViewOriginalText()), getTableType(viewDefinition), - firstNonNull(viewDefinition.getParameters(), ImmutableMap.of()), + getTableParameters(viewDefinition), Optional.ofNullable(viewDefinition.getOwner())); } @@ -819,7 +819,7 @@ public List listMaterializedViews(ConnectorSession session, Opt stats.getGetTables()) .map(GetTablesResult::getTableList) .flatMap(List::stream) - .filter(table -> isTrinoMaterializedView(getTableType(table), firstNonNull(table.getParameters(), ImmutableMap.of()))) + .filter(table -> isTrinoMaterializedView(getTableType(table), getTableParameters(table))) .map(table -> new SchemaTableName(glueNamespace, table.getName())) .collect(toImmutableList())); } @@ -845,7 +845,7 @@ public void createMaterializedView( Optional existing = getTable(session, viewName); if (existing.isPresent()) { - if (!isTrinoMaterializedView(getTableType(existing.get()), firstNonNull(existing.get().getParameters(), ImmutableMap.of()))) { + if (!isTrinoMaterializedView(getTableType(existing.get()), getTableParameters(existing.get()))) { throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Existing table is not a Materialized View: " + viewName); } if (!replace) { @@ -897,7 +897,7 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN com.amazonaws.services.glue.model.Table view = getTable(session, viewName) .orElseThrow(() -> new MaterializedViewNotFoundException(viewName)); - if (!isTrinoMaterializedView(getTableType(view), firstNonNull(view.getParameters(), ImmutableMap.of()))) { + if (!isTrinoMaterializedView(getTableType(view), getTableParameters(view))) { throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Not a Materialized View: " + view.getDatabaseName() + "." + view.getName()); } materializedViewCache.remove(viewName); @@ -907,7 +907,7 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN private void dropStorageTable(ConnectorSession session, com.amazonaws.services.glue.model.Table view) { - Map parameters = firstNonNull(view.getParameters(), ImmutableMap.of()); + Map parameters = getTableParameters(view); String storageTableName = parameters.get(STORAGE_TABLE); if (storageTableName != null) { String storageSchema = Optional.ofNullable(parameters.get(STORAGE_SCHEMA)) @@ -940,7 +940,7 @@ protected Optional doGetMaterializedView(Co } com.amazonaws.services.glue.model.Table table = maybeTable.get(); - if (!isTrinoMaterializedView(getTableType(table), firstNonNull(table.getParameters(), ImmutableMap.of()))) { + if (!isTrinoMaterializedView(getTableType(table), getTableParameters(table))) { return Optional.empty(); } @@ -952,7 +952,7 @@ private Optional createMaterializedViewDefi SchemaTableName viewName, com.amazonaws.services.glue.model.Table table) { - Map materializedViewParameters = firstNonNull(table.getParameters(), ImmutableMap.of()); + Map materializedViewParameters = getTableParameters(table); String storageTable = materializedViewParameters.get(STORAGE_TABLE); checkState(storageTable != null, "Storage table missing in definition of materialized view " + viewName); String storageSchema = Optional.ofNullable(materializedViewParameters.get(STORAGE_SCHEMA)) @@ -991,10 +991,11 @@ public void renameMaterializedView(ConnectorSession session, SchemaTableName sou com.amazonaws.services.glue.model.Table glueTable = getTable(session, source) .orElseThrow(() -> new TableNotFoundException(source)); materializedViewCache.remove(source); - if (!isTrinoMaterializedView(getTableType(glueTable), firstNonNull(glueTable.getParameters(), ImmutableMap.of()))) { + Map tableParameters = getTableParameters(glueTable); + if (!isTrinoMaterializedView(getTableType(glueTable), tableParameters)) { throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Not a Materialized View: " + source); } - TableInput tableInput = getMaterializedViewTableInput(target.getTableName(), glueTable.getViewOriginalText(), glueTable.getOwner(), glueTable.getParameters()); + TableInput tableInput = getMaterializedViewTableInput(target.getTableName(), glueTable.getViewOriginalText(), glueTable.getOwner(), tableParameters); CreateTableRequest createTableRequest = new CreateTableRequest() .withDatabaseName(target.getSchemaName()) .withTableInput(tableInput); @@ -1041,7 +1042,7 @@ public Optional redirectTable(ConnectorSession session, if (table.isEmpty() || VIRTUAL_VIEW.name().equals(getTableTypeNullable(table.get()))) { return Optional.empty(); } - if (!isIcebergTable(firstNonNull(table.get().getParameters(), ImmutableMap.of()))) { + if (!isIcebergTable(getTableParameters(table.get()))) { // After redirecting, use the original table name, with "$partitions" and similar suffixes return targetCatalogName.map(catalog -> new CatalogSchemaTableName(catalog, tableName)); } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java index 1c9f33836604..97e9538b8369 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java @@ -51,6 +51,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.iceberg.IcebergTestUtils.checkParquetFileSorting; import static io.trino.testing.TestingConnectorSession.SESSION; import static io.trino.testing.TestingNames.randomNameSuffix; @@ -214,9 +215,8 @@ protected String getMetadataLocation(String tableName) GetTableRequest getTableRequest = new GetTableRequest() .withDatabaseName(schemaName) .withName(tableName); - return glueClient.getTable(getTableRequest) - .getTable() - .getParameters().get("metadata_location"); + return getTableParameters(glueClient.getTable(getTableRequest).getTable()) + .get("metadata_location"); } @Override diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java index 1bd91d68e448..1ce89a8284d9 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java @@ -41,6 +41,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Iterables.getOnlyElement; import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults; +import static io.trino.plugin.hive.metastore.glue.converter.GlueToTrinoConverter.getTableParameters; import static io.trino.plugin.iceberg.catalog.glue.GlueIcebergUtil.getTableInput; import static io.trino.testing.TestingNames.randomNameSuffix; import static org.assertj.core.api.Assertions.assertThat; @@ -112,7 +113,7 @@ public void testNotRemoveExistingArchive() // Add a new archive using Glue client Table glueTable = glueClient.getTable(new GetTableRequest().withDatabaseName(schemaName).withName(table.getName())).getTable(); - TableInput tableInput = getTableInput(table.getName(), Optional.empty(), glueTable.getParameters()); + TableInput tableInput = getTableInput(table.getName(), Optional.empty(), getTableParameters(glueTable)); glueClient.updateTable(new UpdateTableRequest().withDatabaseName(schemaName).withTableInput(tableInput)); assertThat(getTableVersions(schemaName, table.getName())).hasSize(2);