From 9151dc87ab05348c55d800262acdb1eb2eb11f37 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 19 Jun 2023 17:23:56 +0200 Subject: [PATCH 1/9] Avoid creating Hive tables with double slashed location Hive connector does not support table locations containing double slashes. On S3 this leads to correctness issues (e.g. INSERT works, but SELECT does not find any data). This commit - restores normalization of implicit table location during CREATE TABLE. There used to be such normalization until 8bd9f754fb2aec61ffa353adfb421b460b60d947. - rejects explicit table locations containing double slash during `CREATE TABLE .. WITH (external_location = ...)`. Before 8bd9f754fb2aec61ffa353adfb421b460b60d947 there used to be normalization also during this flow, but rejecting such unsupported locations is deemed more correct. --- .../io/trino/plugin/hive/HiveMetadata.java | 14 +++++- .../plugin/hive/util/HiveWriteUtils.java | 5 ++- .../hive/TestHiveS3AndGlueMetastoreTest.java | 45 ++++++++++++------- 3 files changed, 46 insertions(+), 18 deletions(-) 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 564f128dd269..68d41e198c7b 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 @@ -1275,12 +1275,24 @@ private static String validateAvroSchemaLiteral(String avroSchemaLiteral) private static Location getValidatedExternalLocation(String location) { + Location validated; try { - return Location.of(location); + validated = Location.of(location); } catch (IllegalArgumentException e) { throw new TrinoException(INVALID_TABLE_PROPERTY, "External location is not a valid file system URI: " + location, e); } + + // TODO (https://github.com/trinodb/trino/issues/17803) We cannot accept locations with double slash until all relevant Hive connector components are migrated off Hadoop Path. + // Hadoop Path "normalizes location", e.g.: + // - removes double slashes (such locations are rejected), + // - removes trailing slash (such locations are accepted; foo/bar and foo/bar/ are treated as equivalent, and rejecting locations with trailing slash could pose UX issues) + // - replaces file:/// with file:/ (such locations are accepted). + if (validated.path().contains("//")) { + throw new TrinoException(INVALID_TABLE_PROPERTY, "Unsupported location that cannot be internally represented: " + location); + } + + return validated; } private void checkExternalPath(HdfsContext context, Path path) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveWriteUtils.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveWriteUtils.java index b6c061ea4775..89152c58c3f4 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveWriteUtils.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveWriteUtils.java @@ -453,7 +453,10 @@ public static Location getTableDefaultLocation(Database database, HdfsContext co } } - return Location.of(location).appendPath(escapeTableName(tableName)); + // Note: this results in `databaseLocation` being a "normalized location", e.g. not containing double slashes. + // TODO (https://github.com/trinodb/trino/issues/17803): We need to use normalized location until all relevant Hive connector components are migrated off Hadoop Path. + Location databaseLocation = Location.of(databasePath.toString()); + return databaseLocation.appendPath(escapeTableName(tableName)); } public static boolean pathExists(HdfsContext context, HdfsEnvironment hdfsEnvironment, Path path) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index e0f2b5d797af..4df2b89396ac 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -24,11 +24,9 @@ import java.nio.file.Path; import java.util.HashSet; -import java.util.List; import java.util.Optional; import java.util.Set; -import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.plugin.hive.metastore.glue.GlueHiveMetastore.createTestingGlueHiveMetastore; import static io.trino.spi.security.SelectedRole.Type.ROLE; import static io.trino.testing.TestingNames.randomNameSuffix; @@ -123,9 +121,14 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, St String location = locationPattern.formatted(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? ",partitioned_by = ARRAY['col_int']" : ""); - assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + + String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + - "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; + if (locationPattern.contains("double_slash")) { + assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); + return; + } + assertUpdate(create, 3); assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); String actualTableLocation = getTableLocation(tableName); @@ -154,7 +157,9 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, S assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int int)" + partitionQueryPart); - String expectedTableLocation = (schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName; + String expectedTableLocation = ((schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName) + // Hive normalizes double slash + .replaceAll("(? super.testOptimizeWithProvidedTableLocation(partitioned, locationPattern)) + .hasMessageStartingWith("Unsupported location that cannot be internally represented: ") + .hasStackTraceContaining("SQL: CREATE TABLE test_optimize_"); + return; + } + super.testOptimizeWithProvidedTableLocation(partitioned, locationPattern); + } + @Test(dataProvider = "locationPatternsDataProvider") public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String locationPattern) { @@ -186,9 +203,14 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String loc String location = locationPattern.formatted(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? ",partitioned_by = ARRAY['col_int']" : ""); - assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + + String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + - "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; + if (locationPattern.contains("double_slash")) { + assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); + return; + } + assertUpdate(create, 3); assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); @@ -286,13 +308,4 @@ public void testSchemaNameEscape() assertUpdate("DROP SCHEMA \"" + schemaName + "\""); } - - @Override - protected List locationPatterns() - { - // TODO https://github.com/trinodb/trino/issues/17803 Fix correctness issue with double slash - return super.locationPatterns().stream() - .filter(location -> !location.contains("double_slash")) - .collect(toImmutableList()); - } } From a86b0fba3c0e7009a6919870078e73bd70f4e5bb Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 17:34:10 +0200 Subject: [PATCH 2/9] Configure TestIcebergS3AndGlueMetastoreTest as other similar tests Configure bucket used in `TestIcebergS3AndGlueMetastoreTest` same way as in other `BaseS3AndGlueMetastoreTest` subclasses. Makes it easier to run tests locally. This also changes `TestIcebergGlueCatalogConnectorSmokeTest` for consistency. --- .github/workflows/ci.yml | 1 - .../glue/TestIcebergGlueCatalogConnectorSmokeTest.java | 6 ++---- .../catalog/glue/TestIcebergS3AndGlueMetastoreTest.java | 7 +++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce5312fe5be7..f0d5396a038c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -715,7 +715,6 @@ jobs: (env.CI_SKIP_SECRETS_PRESENCE_CHECKS != '' || env.AWS_ACCESS_KEY_ID != '' || env.AWS_SECRET_ACCESS_KEY != '' || env.GCP_CREDENTIALS_KEY != '') run: | $MAVEN test ${MAVEN_TEST} -pl :trino-iceberg ${{ format('-P {0}', matrix.profile) }} \ - -Ds3.bucket=${S3_BUCKET} \ -Dtesting.gcp-storage-bucket="trino-ci-test-us-east" \ -Dtesting.gcp-credentials-key="${GCP_CREDENTIALS_KEY}" \ -Dhive.hadoop2.azure-abfs-container="${ABFS_CONTAINER}" \ 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 76daa56560ff..247be6409f18 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 @@ -43,7 +43,6 @@ import io.trino.testing.QueryRunner; import org.apache.iceberg.FileFormat; import org.testng.annotations.AfterClass; -import org.testng.annotations.Parameters; import org.testng.annotations.Test; import java.util.List; @@ -72,11 +71,10 @@ public class TestIcebergGlueCatalogConnectorSmokeTest private final AWSGlueAsync glueClient; private final TrinoFileSystemFactory fileSystemFactory; - @Parameters("s3.bucket") - public TestIcebergGlueCatalogConnectorSmokeTest(String bucketName) + public TestIcebergGlueCatalogConnectorSmokeTest() { super(FileFormat.PARQUET); - this.bucketName = requireNonNull(bucketName, "bucketName is null"); + this.bucketName = requireNonNull(System.getenv("S3_BUCKET"), "Environment S3_BUCKET was not set"); this.schemaName = "test_iceberg_smoke_" + randomNameSuffix(); glueClient = AWSGlueAsyncClientBuilder.defaultClient(); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index aae36ebc3095..84b9de648439 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -19,7 +19,6 @@ import io.trino.plugin.iceberg.IcebergQueryRunner; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; -import org.testng.annotations.Parameters; import org.testng.annotations.Test; import java.nio.file.Path; @@ -28,16 +27,16 @@ import static io.trino.plugin.hive.metastore.glue.GlueHiveMetastore.createTestingGlueHiveMetastore; import static io.trino.testing.TestingNames.randomNameSuffix; +import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestIcebergS3AndGlueMetastoreTest extends BaseS3AndGlueMetastoreTest { - @Parameters("s3.bucket") - public TestIcebergS3AndGlueMetastoreTest(String bucketName) + public TestIcebergS3AndGlueMetastoreTest() { - super("partitioning", "location", bucketName); + super("partitioning", "location", requireNonNull(System.getenv("S3_BUCKET"), "Environment S3_BUCKET was not set")); } @Override From 240721ef1d239410670d5eb0e62650b1721bbefd Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 17:22:58 +0200 Subject: [PATCH 3/9] Do not prune file system cache This may be needed when working with MinIO containers where bucket names can be reused. This is not needed when working with real S3 where bucket names are unique. Also, it's unclear whether this is a safe operation when tests execute in parallel. --- .../metastore/glue/TestDeltaS3AndGlueMetastoreTest.java | 3 --- .../io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java | 3 --- .../catalog/glue/TestIcebergS3AndGlueMetastoreTest.java | 3 --- 3 files changed, 9 deletions(-) diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java index 533b87b749c2..cc12d940b2ee 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java @@ -14,7 +14,6 @@ package io.trino.plugin.deltalake.metastore.glue; import com.google.common.collect.ImmutableMap; -import io.trino.hdfs.TrinoFileSystemCache; import io.trino.plugin.deltalake.DeltaLakeQueryRunner; import io.trino.plugin.hive.BaseS3AndGlueMetastoreTest; import io.trino.testing.DistributedQueryRunner; @@ -42,8 +41,6 @@ public TestDeltaS3AndGlueMetastoreTest() protected QueryRunner createQueryRunner() throws Exception { - closeAfterClass(TrinoFileSystemCache.INSTANCE::closeAll); - metastore = createTestingGlueHiveMetastore(Path.of(schemaPath())); DistributedQueryRunner queryRunner = DeltaLakeQueryRunner.builder() .setCatalogName(DELTA_CATALOG) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index 4df2b89396ac..73b73df62531 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -15,7 +15,6 @@ import com.google.common.collect.ImmutableMap; import io.trino.Session; -import io.trino.hdfs.TrinoFileSystemCache; import io.trino.spi.security.Identity; import io.trino.spi.security.SelectedRole; import io.trino.testing.DistributedQueryRunner; @@ -47,8 +46,6 @@ public TestHiveS3AndGlueMetastoreTest() protected QueryRunner createQueryRunner() throws Exception { - closeAfterClass(TrinoFileSystemCache.INSTANCE::closeAll); - metastore = createTestingGlueHiveMetastore(Path.of(schemaPath())); Session session = createSession(Optional.of(new SelectedRole(ROLE, Optional.of("admin")))); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index 84b9de648439..b45e52ef55ee 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -14,7 +14,6 @@ package io.trino.plugin.iceberg.catalog.glue; import com.google.common.collect.ImmutableMap; -import io.trino.hdfs.TrinoFileSystemCache; import io.trino.plugin.hive.BaseS3AndGlueMetastoreTest; import io.trino.plugin.iceberg.IcebergQueryRunner; import io.trino.testing.DistributedQueryRunner; @@ -43,8 +42,6 @@ public TestIcebergS3AndGlueMetastoreTest() protected QueryRunner createQueryRunner() throws Exception { - closeAfterClass(TrinoFileSystemCache.INSTANCE::closeAll); - metastore = createTestingGlueHiveMetastore(Path.of(schemaPath())); DistributedQueryRunner queryRunner = IcebergQueryRunner.builder() .setIcebergProperties(ImmutableMap.builder() From 5e70149b65d38f52f4be30a9c382ba802666e05b Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 09:28:16 +0200 Subject: [PATCH 4/9] Code cleanup --- .../plugin/hive/BaseS3AndGlueMetastoreTest.java | 4 ++-- .../hive/TestHiveS3AndGlueMetastoreTest.java | 4 ++-- .../glue/TestIcebergS3AndGlueMetastoreTest.java | 16 +++++++--------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java index 130852a5368e..627a5c972a8e 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java @@ -250,9 +250,9 @@ protected void validateFilesAfterOptimize(String location, Set initialFi protected abstract void validateMetadataFiles(String location); - protected void validateTableLocation(String tableName, String location) + protected void validateTableLocation(String tableName, String expectedLocation) { - assertThat(getTableLocation(tableName)).isEqualTo(location); + assertThat(getTableLocation(tableName)).isEqualTo(expectedLocation); } protected void validateFilesAfterDrop(String location) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index 73b73df62531..34cb9632b93f 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -212,7 +212,7 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String loc assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); - //Check statistics collection on write + // Check statistics collection on write if (partitioned) { assertQuery("SHOW STATS FOR " + tableName, """ VALUES @@ -228,7 +228,7 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String loc (null, null, null, null, 4.0, null, null)"""); } - //Check statistics collection explicitly + // Check statistics collection explicitly assertUpdate("ANALYZE " + tableName, 4); if (partitioned) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index b45e52ef55ee..0de6bd41a363 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -77,15 +77,13 @@ protected void validateMetadataFiles(String location) } @Override - protected void validateTableLocation(String tableName, String location) + protected void validateTableLocation(String tableName, String expectedLocation) { - if (location.endsWith("/")) { - //Iceberg removes trailing slash from location, and it's expected. - assertThat(getTableLocation(tableName) + "/").isEqualTo(location); - } - else { - assertThat(getTableLocation(tableName)).isEqualTo(location); + // Iceberg removes trailing slash from location, and it's expected. + if (expectedLocation.endsWith("/")) { + expectedLocation = expectedLocation.replaceFirst("/$", ""); } + assertThat(getTableLocation(tableName)).isEqualTo(expectedLocation); } private Set getAllMetadataDataFilesFromTableDirectory(String tableLocation) @@ -123,12 +121,12 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String loc ('col_int', null, 4.0, 0.0, null, 1, 4), (null, null, null, null, 4.0, null, null)"""; - //Check extended statistics collection on write + // Check extended statistics collection on write assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); // drop stats assertUpdate("ALTER TABLE " + tableName + " EXECUTE DROP_EXTENDED_STATS"); - //Check extended statistics collection explicitly + // Check extended statistics collection explicitly assertUpdate("ANALYZE " + tableName); assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); From 72325aee0786bef10ece90b352155ea44862a2d0 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 10:00:49 +0200 Subject: [PATCH 5/9] Describe test location patterns with enum The `String.contains`-based inclusions/exclusions doesn't allow distinguishing between hypothetical cases like "trailing_slash" and "two_trailing_slashes". This also simplifies generated test locations for schemas. Previously the schema name was formatted twice into the location string. --- .../hive/BaseS3AndGlueMetastoreTest.java | 60 ++++++++++++------- .../hive/TestHiveS3AndGlueMetastoreTest.java | 23 +++---- .../TestIcebergS3AndGlueMetastoreTest.java | 4 +- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java index 627a5c972a8e..238125ffd3d6 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java @@ -17,7 +17,6 @@ import com.amazonaws.services.s3.AmazonS3ClientBuilder; import com.amazonaws.services.s3.model.ListObjectsV2Request; import com.amazonaws.services.s3.model.S3ObjectSummary; -import com.google.common.collect.ImmutableList; import io.trino.Session; import io.trino.plugin.hive.metastore.HiveMetastore; import io.trino.spi.connector.SchemaNotFoundException; @@ -32,6 +31,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.google.common.base.Verify.verify; import static com.google.common.collect.Sets.union; @@ -85,26 +85,14 @@ public void tearDown() @DataProvider public Object[][] locationPatternsDataProvider() { - return cartesianProduct(trueFalse(), locationPatterns().stream().collect(toDataProvider())); - } - - protected List locationPatterns() - { - return ImmutableList.builder() - .add("s3://%s/%s/regular/%s") - .add("s3://%s/%s/trailing_slash/%s/") - .add("s3://%s/%s//double_slash/%s") - .add("s3://%s/%s/a%%percent/%s") - .add("s3://%s/%s/a whitespace/%s") - .add("s3://%s/%s/trailing_whitespace/%s ") - .build(); + return cartesianProduct(trueFalse(), Stream.of(LocationPattern.values()).collect(toDataProvider())); } @Test(dataProvider = "locationPatternsDataProvider") - public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_basic_operations_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? "," + partitionByKeyword + " = ARRAY['col_str']" : ""); assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + @@ -131,10 +119,10 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, St } @Test(dataProvider = "locationPatternsDataProvider") - public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, String locationPattern) + public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, LocationPattern locationPattern) { String schemaName = "test_basic_operations_schema_" + randomNameSuffix(); - String schemaLocation = locationPattern.formatted(bucketName, schemaName, schemaName); + String schemaLocation = locationPattern.locationForSchema(bucketName, schemaName); String tableName = "test_basic_operations_table_" + randomNameSuffix(); String qualifiedTableName = schemaName + "." + tableName; String partitionQueryPart = (partitioned ? "WITH (" + partitionByKeyword + " = ARRAY['col_str'])" : ""); @@ -169,10 +157,10 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, S } @Test(dataProvider = "locationPatternsDataProvider") - public void testMergeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testMergeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_merge_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? "," + partitionByKeyword + " = ARRAY['col_str']" : ""); assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + @@ -201,10 +189,10 @@ public void testMergeWithProvidedTableLocation(boolean partitioned, String locat } @Test(dataProvider = "locationPatternsDataProvider") - public void testOptimizeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testOptimizeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_optimize_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? "," + partitionByKeyword + " = ARRAY['value']" : ""); String locationQueryPart = locationKeyword + "= '" + location + "'"; @@ -313,4 +301,32 @@ protected void verifyPathExist(String path) { assertThat(s3Path(s3, path)).exists(); } + + protected enum LocationPattern + { + REGULAR("s3://%s/%s/regular/%s"), + TRAILING_SLASH("s3://%s/%s/trailing_slash/%s/"), + DOUBLE_SLASH("s3://%s/%s//double_slash/%s"), + PERCENT("s3://%s/%s/a%%percent/%s"), + WHITESPACE("s3://%s/%s/a whitespace/%s"), + TRAILING_WHITESPACE("s3://%s/%s/trailing_whitespace/%s "), + /**/; + + private final String locationPattern; + + LocationPattern(String locationPattern) + { + this.locationPattern = requireNonNull(locationPattern, "locationPattern is null"); + } + + public String locationForSchema(String bucketName, String schemaName) + { + return locationPattern.formatted(bucketName, "warehouse", schemaName); + } + + public String locationForTable(String bucketName, String schemaName, String tableName) + { + return locationPattern.formatted(bucketName, schemaName, tableName); + } + } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index 34cb9632b93f..eb12f9e0ab30 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -26,6 +26,7 @@ import java.util.Optional; import java.util.Set; +import static io.trino.plugin.hive.BaseS3AndGlueMetastoreTest.LocationPattern.DOUBLE_SLASH; import static io.trino.plugin.hive.metastore.glue.GlueHiveMetastore.createTestingGlueHiveMetastore; import static io.trino.spi.security.SelectedRole.Type.ROLE; import static io.trino.testing.TestingNames.randomNameSuffix; @@ -112,16 +113,16 @@ protected void validateFilesAfterOptimize(String location, Set initialFi @Override // Row-level modifications are not supported for Hive tables @Test(dataProvider = "locationPatternsDataProvider") - public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_basic_operations_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? ",partitioned_by = ARRAY['col_int']" : ""); String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; - if (locationPattern.contains("double_slash")) { + if (locationPattern == DOUBLE_SLASH) { assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); return; } @@ -142,10 +143,10 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, St @Override // Row-level modifications are not supported for Hive tables @Test(dataProvider = "locationPatternsDataProvider") - public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, String locationPattern) + public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, LocationPattern locationPattern) { String schemaName = "test_basic_operations_schema_" + randomNameSuffix(); - String schemaLocation = locationPattern.formatted(bucketName, schemaName, schemaName); + String schemaLocation = locationPattern.locationForSchema(bucketName, schemaName); String tableName = "test_basic_operations_table_" + randomNameSuffix(); String qualifiedTableName = schemaName + "." + tableName; String partitionQueryPart = (partitioned ? " WITH (partitioned_by = ARRAY['col_int'])" : ""); @@ -176,15 +177,15 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, S @Override @Test(dataProvider = "locationPatternsDataProvider") - public void testMergeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testMergeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { // Row-level modifications are not supported for Hive tables } @Override - public void testOptimizeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testOptimizeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { - if (locationPattern.contains("double_slash")) { + if (locationPattern == DOUBLE_SLASH) { assertThatThrownBy(() -> super.testOptimizeWithProvidedTableLocation(partitioned, locationPattern)) .hasMessageStartingWith("Unsupported location that cannot be internally represented: ") .hasStackTraceContaining("SQL: CREATE TABLE test_optimize_"); @@ -194,16 +195,16 @@ public void testOptimizeWithProvidedTableLocation(boolean partitioned, String lo } @Test(dataProvider = "locationPatternsDataProvider") - public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testAnalyzeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_analyze_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? ",partitioned_by = ARRAY['col_int']" : ""); String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; - if (locationPattern.contains("double_slash")) { + if (locationPattern == DOUBLE_SLASH) { assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); return; } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index 0de6bd41a363..a89087c0b2f2 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -102,10 +102,10 @@ protected Set getAllDataFilesFromTableDirectory(String tableLocation) } @Test(dataProvider = "locationPatternsDataProvider") - public void testAnalyzeWithProvidedTableLocation(boolean partitioned, String locationPattern) + public void testAnalyzeWithProvidedTableLocation(boolean partitioned, LocationPattern locationPattern) { String tableName = "test_analyze_" + randomNameSuffix(); - String location = locationPattern.formatted(bucketName, schemaName, tableName); + String location = locationPattern.locationForTable(bucketName, schemaName, tableName); String partitionQueryPart = (partitioned ? ",partitioning = ARRAY['col_str']" : ""); assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + From 05ddda8a09df7f1e4b366d47ee25688024f65880 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 11:11:29 +0200 Subject: [PATCH 6/9] Remove unused and defunct TestTable constructor Remove constructor which doesn't construct the table. Since it was randomizing the name, it couldn't even be used for dropping existing table. --- .../src/main/java/io/trino/testing/sql/TestTable.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/testing/trino-testing/src/main/java/io/trino/testing/sql/TestTable.java b/testing/trino-testing/src/main/java/io/trino/testing/sql/TestTable.java index 8f80948ab500..4e125cdd4ef9 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/sql/TestTable.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/sql/TestTable.java @@ -44,13 +44,6 @@ public TestTable(SqlExecutor sqlExecutor, String namePrefix, String tableDefinit createAndInsert(rowsToInsert); } - public TestTable(SqlExecutor sqlExecutor, String namePrefix) - { - this.sqlExecutor = sqlExecutor; - this.name = namePrefix + randomNameSuffix(); - this.tableDefinition = null; - } - public void createAndInsert(List rowsToInsert) { sqlExecutor.execute(format("CREATE TABLE %s %s", name, tableDefinition)); From 705ffddb413fd85fbb5da6859a769253017ef5e4 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 11:22:07 +0200 Subject: [PATCH 7/9] Ensure test resource cleanup in BaseS3AndGlueMetastoreTest Use try with resources to ensure that test tables are dropped even if an assertion fails. --- .../hive/BaseS3AndGlueMetastoreTest.java | 126 ++++++++------- .../hive/TestHiveS3AndGlueMetastoreTest.java | 146 +++++++++--------- .../TestIcebergS3AndGlueMetastoreTest.java | 53 ++++--- 3 files changed, 168 insertions(+), 157 deletions(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java index 238125ffd3d6..f99e18205552 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java @@ -21,6 +21,7 @@ import io.trino.plugin.hive.metastore.HiveMetastore; import io.trino.spi.connector.SchemaNotFoundException; import io.trino.testing.AbstractTestQueryFramework; +import org.intellij.lang.annotations.Language; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; @@ -98,23 +99,23 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, Lo assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); - validateTableLocation(tableName, location); + try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); + validateTableLocation(tableName, location); - assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); + assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); - assertUpdate("UPDATE " + tableName + " SET col_str = 'other' WHERE col_int = 2", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3), ('str4', 4)"); + assertUpdate("UPDATE " + tableName + " SET col_str = 'other' WHERE col_int = 2", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3), ('str4', 4)"); - assertUpdate("DELETE FROM " + tableName + " WHERE col_int = 3", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str4', 4)"); + assertUpdate("DELETE FROM " + tableName + " WHERE col_int = 3", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str4', 4)"); - assertThat(getTableFiles(location)).isNotEmpty(); - validateDataFiles(partitioned ? "col_str" : "", tableName, location); - validateMetadataFiles(location); - - assertUpdate("DROP TABLE " + tableName); + assertThat(getTableFiles(location)).isNotEmpty(); + validateDataFiles(partitioned ? "col_str" : "", tableName, location); + validateMetadataFiles(location); + } validateFilesAfterDrop(location); } @@ -127,32 +128,33 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L String qualifiedTableName = schemaName + "." + tableName; String partitionQueryPart = (partitioned ? "WITH (" + partitionByKeyword + " = ARRAY['col_str'])" : ""); + String actualTableLocation; assertUpdate("CREATE SCHEMA " + schemaName + " WITH (location = '" + schemaLocation + "')"); - assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); - - assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_int int, col_str varchar)" + partitionQueryPart); - // in case of regular CREATE TABLE, location has generated suffix - String expectedTableLocationPattern = (schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName + "-[a-z0-9]+"; - String actualTableLocation = getTableLocation(qualifiedTableName); - assertThat(actualTableLocation).matches(expectedTableLocationPattern); - - assertUpdate("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); - assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); - - assertUpdate("UPDATE " + qualifiedTableName + " SET col_str = 'other' WHERE col_int = 2", 1); - assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3)"); - - assertUpdate("DELETE FROM " + qualifiedTableName + " WHERE col_int = 3", 1); - assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('other', 2)"); - - assertThat(getTableFiles(actualTableLocation)).isNotEmpty(); - validateDataFiles(partitioned ? "col_str" : "", qualifiedTableName, actualTableLocation); - validateMetadataFiles(actualTableLocation); - - assertUpdate("DROP TABLE " + qualifiedTableName); - assertThat(getTableFiles(actualTableLocation)).isEmpty(); - - assertUpdate("DROP SCHEMA " + schemaName); + try (UncheckedCloseable ignoredDropSchema = onClose("DROP SCHEMA " + schemaName)) { + assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); + + assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_int int, col_str varchar)" + partitionQueryPart); + try (UncheckedCloseable ignoredDropTable = onClose("DROP TABLE " + qualifiedTableName)) { + // in case of regular CREATE TABLE, location has generated suffix + String expectedTableLocationPattern = (schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName + "-[a-z0-9]+"; + actualTableLocation = getTableLocation(qualifiedTableName); + assertThat(actualTableLocation).matches(expectedTableLocationPattern); + + assertUpdate("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); + assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); + + assertUpdate("UPDATE " + qualifiedTableName + " SET col_str = 'other' WHERE col_int = 2", 1); + assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3)"); + + assertUpdate("DELETE FROM " + qualifiedTableName + " WHERE col_int = 3", 1); + assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('other', 2)"); + + assertThat(getTableFiles(actualTableLocation)).isNotEmpty(); + validateDataFiles(partitioned ? "col_str" : "", qualifiedTableName, actualTableLocation); + validateMetadataFiles(actualTableLocation); + } + assertThat(getTableFiles(actualTableLocation)).isEmpty(); + } assertThat(getTableFiles(actualTableLocation)).isEmpty(); } @@ -166,25 +168,25 @@ public void testMergeWithProvidedTableLocation(boolean partitioned, LocationPatt assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); - - assertUpdate("MERGE INTO " + tableName + " USING (VALUES 1) t(x) ON false" + - " WHEN NOT MATCHED THEN INSERT VALUES ('str4', 4)", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); + try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); - assertUpdate("MERGE INTO " + tableName + " USING (VALUES 2) t(x) ON col_int = x" + - " WHEN MATCHED THEN UPDATE SET col_str = 'other'", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3), ('str4', 4)"); + assertUpdate("MERGE INTO " + tableName + " USING (VALUES 1) t(x) ON false" + + " WHEN NOT MATCHED THEN INSERT VALUES ('str4', 4)", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); - assertUpdate("MERGE INTO " + tableName + " USING (VALUES 3) t(x) ON col_int = x" + - " WHEN MATCHED THEN DELETE", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str4', 4)"); + assertUpdate("MERGE INTO " + tableName + " USING (VALUES 2) t(x) ON col_int = x" + + " WHEN MATCHED THEN UPDATE SET col_str = 'other'", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str3', 3), ('str4', 4)"); - assertThat(getTableFiles(location)).isNotEmpty(); - validateDataFiles(partitioned ? "col_str" : "", tableName, location); - validateMetadataFiles(location); + assertUpdate("MERGE INTO " + tableName + " USING (VALUES 3) t(x) ON col_int = x" + + " WHEN MATCHED THEN DELETE", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('other', 2), ('str4', 4)"); - assertUpdate("DROP TABLE " + tableName); + assertThat(getTableFiles(location)).isNotEmpty(); + validateDataFiles(partitioned ? "col_str" : "", tableName, location); + validateMetadataFiles(location); + } validateFilesAfterDrop(location); } @@ -198,7 +200,7 @@ public void testOptimizeWithProvidedTableLocation(boolean partitioned, LocationP assertUpdate("CREATE TABLE " + tableName + " (key integer, value varchar) " + "WITH (" + locationQueryPart + partitionQueryPart + ")"); - try { + try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { // create multiple data files, INSERT with multiple values would create only one file (if not partitioned) assertUpdate("INSERT INTO " + tableName + " VALUES (1, 'one')", 1); assertUpdate("INSERT INTO " + tableName + " VALUES (2, 'a//double_slash')", 1); @@ -218,9 +220,6 @@ public void testOptimizeWithProvidedTableLocation(boolean partitioned, LocationP Set updatedFiles = getActiveFiles(tableName); validateFilesAfterOptimize(getTableLocation(tableName), initialFiles, updatedFiles); } - finally { - assertUpdate("DROP TABLE " + tableName); - } } protected Session sessionForOptimize() @@ -292,6 +291,12 @@ protected List getTableFiles(String location) .toList(); } + protected UncheckedCloseable onClose(@Language("SQL") String sql) + { + requireNonNull(sql, "sql is null"); + return () -> assertUpdate(sql); + } + protected String schemaPath() { return "s3://%s/%s".formatted(bucketName, schemaName); @@ -329,4 +334,11 @@ public String locationForTable(String bucketName, String schemaName, String tabl return locationPattern.formatted(bucketName, schemaName, tableName); } } + + protected interface UncheckedCloseable + extends AutoCloseable + { + @Override + void close(); + } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index eb12f9e0ab30..2b795e340f47 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -127,18 +127,18 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, Lo return; } assertUpdate(create, 3); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); + try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); - String actualTableLocation = getTableLocation(tableName); - assertThat(actualTableLocation).isEqualTo(location); + String actualTableLocation = getTableLocation(tableName); + assertThat(actualTableLocation).isEqualTo(location); - assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); + assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); - assertThat(getTableFiles(actualTableLocation)).isNotEmpty(); - validateDataFiles(partitioned ? "col_int" : "", tableName, actualTableLocation); - - assertUpdate("DROP TABLE " + tableName); + assertThat(getTableFiles(actualTableLocation)).isNotEmpty(); + validateDataFiles(partitioned ? "col_int" : "", tableName, actualTableLocation); + } } @Override // Row-level modifications are not supported for Hive tables @@ -151,27 +151,28 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L String qualifiedTableName = schemaName + "." + tableName; String partitionQueryPart = (partitioned ? " WITH (partitioned_by = ARRAY['col_int'])" : ""); + String actualTableLocation; assertUpdate("CREATE SCHEMA " + schemaName + " WITH (location = '" + schemaLocation + "')"); - assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); - - assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int int)" + partitionQueryPart); - String expectedTableLocation = ((schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName) - // Hive normalizes double slash - .replaceAll("(? assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str, col_int) AS VALUES ('str1', 1)")) - .hasMessageContaining("Fragment is not allowed in a file system location"); + assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str, col_int) AS VALUES ('str1', 1)")) + .hasMessageContaining("Fragment is not allowed in a file system location"); - assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int integer)")) - .hasMessageContaining("Fragment is not allowed in a file system location"); - - assertUpdate("DROP SCHEMA " + schemaName); + assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int integer)")) + .hasMessageContaining("Fragment is not allowed in a file system location"); + } } @Test @@ -302,8 +302,8 @@ public void testSchemaNameEscape() String tableName = "test_table_schema_escaped_" + randomNameSuffix(); assertUpdate("CREATE SCHEMA \"%2$s\" WITH (location = 's3://%1$s/%2$s')".formatted(bucketName, schemaName)); - assertQueryFails("CREATE TABLE \"" + schemaName + "\"." + tableName + " (col) AS VALUES 1", "Failed checking path: .*"); - - assertUpdate("DROP SCHEMA \"" + schemaName + "\""); + try (UncheckedCloseable ignored = onClose("DROP SCHEMA \"" + schemaName + "\"")) { + assertQueryFails("CREATE TABLE \"" + schemaName + "\"." + tableName + " (col) AS VALUES 1", "Failed checking path: .*"); + } } } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index a89087c0b2f2..541e4aa032bc 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -111,26 +111,25 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, LocationPa assertUpdate("CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); - - assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); - assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); - - String expectedStatistics = """ - VALUES - ('col_str', null, 4.0, 0.0, null, null, null), - ('col_int', null, 4.0, 0.0, null, 1, 4), - (null, null, null, null, 4.0, null, null)"""; - - // Check extended statistics collection on write - assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); - - // drop stats - assertUpdate("ALTER TABLE " + tableName + " EXECUTE DROP_EXTENDED_STATS"); - // Check extended statistics collection explicitly - assertUpdate("ANALYZE " + tableName); - assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); - - assertUpdate("DROP TABLE " + tableName); + try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { + assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); + assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); + + String expectedStatistics = """ + VALUES + ('col_str', null, 4.0, 0.0, null, null, null), + ('col_int', null, 4.0, 0.0, null, 1, 4), + (null, null, null, null, 4.0, null, null)"""; + + // Check extended statistics collection on write + assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); + + // drop stats + assertUpdate("ALTER TABLE " + tableName + " EXECUTE DROP_EXTENDED_STATS"); + // Check extended statistics collection explicitly + assertUpdate("ANALYZE " + tableName); + assertQuery("SHOW STATS FOR " + tableName, expectedStatistics); + } } @Test @@ -164,14 +163,14 @@ public void testCreateSchemaWithIncorrectLocation() String qualifiedTableName = schemaName + "." + tableName; assertUpdate("CREATE SCHEMA " + schemaName + " WITH (location = '" + schemaLocation + "')"); - assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); + try (UncheckedCloseable ignored = onClose("DROP SCHEMA " + schemaName)) { + assertThat(getSchemaLocation(schemaName)).isEqualTo(schemaLocation); - assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int int)")) - .hasMessageContaining("location contains a fragment"); + assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_str varchar, col_int int)")) + .hasMessageContaining("location contains a fragment"); - assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + " AS SELECT * FROM tpch.tiny.nation")) - .hasMessageContaining("location contains a fragment"); - - assertUpdate("DROP SCHEMA " + schemaName); + assertThatThrownBy(() -> assertUpdate("CREATE TABLE " + qualifiedTableName + " AS SELECT * FROM tpch.tiny.nation")) + .hasMessageContaining("location contains a fragment"); + } } } From b25ebc602874e5ad5e2d35da78d6aca066fd6207 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 09:12:33 +0200 Subject: [PATCH 8/9] Add more double slashes related test cases --- .../hive/BaseS3AndGlueMetastoreTest.java | 36 +++++++++++++++++-- .../hive/TestHiveS3AndGlueMetastoreTest.java | 10 +++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java index f99e18205552..3cc62b6acbe4 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java @@ -36,6 +36,7 @@ import static com.google.common.base.Verify.verify; import static com.google.common.collect.Sets.union; +import static io.trino.plugin.hive.BaseS3AndGlueMetastoreTest.LocationPattern.TWO_TRAILING_SLASHES; import static io.trino.plugin.hive.S3Assert.s3Path; import static io.trino.testing.DataProviders.cartesianProduct; import static io.trino.testing.DataProviders.toDataProvider; @@ -44,6 +45,7 @@ import static java.lang.String.format; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public abstract class BaseS3AndGlueMetastoreTest extends AbstractTestQueryFramework @@ -103,6 +105,12 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, Lo assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); validateTableLocation(tableName, location); + if (locationPattern == TWO_TRAILING_SLASHES && getClass().getName().contains(".deltalake.")) { + // TODO (https://github.com/trinodb/trino/issues/17966): writes fail when Delta table is declared with location ending with two slashes + assertThatThrownBy(() -> query("INSERT INTO " + tableName + " VALUES ('str4', 4)")) + .hasMessageStartingWith("Location does not have parent: "); + return; + } assertUpdate("INSERT INTO " + tableName + " VALUES ('str4', 4)", 1); assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); @@ -140,6 +148,12 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L actualTableLocation = getTableLocation(qualifiedTableName); assertThat(actualTableLocation).matches(expectedTableLocationPattern); + if (locationPattern == TWO_TRAILING_SLASHES && getClass().getName().contains(".deltalake.")) { + // TODO (https://github.com/trinodb/trino/issues/17966): writes fail when Delta table is declared within schema with location ending with two slashes + assertThatThrownBy(() -> query("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)")) + .hasMessageStartingWith("Location does not have parent: "); + return; + } assertUpdate("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); @@ -171,6 +185,13 @@ public void testMergeWithProvidedTableLocation(boolean partitioned, LocationPatt try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)"); + if (locationPattern == TWO_TRAILING_SLASHES && getClass().getName().contains(".deltalake.")) { + // TODO (https://github.com/trinodb/trino/issues/17966): writes fail when Delta table is declared with location ending with two slashes + assertThatThrownBy(() -> query("MERGE INTO " + tableName + " USING (VALUES 1) t(x) ON false" + + " WHEN NOT MATCHED THEN INSERT VALUES ('str4', 4)")) + .hasMessageStartingWith("Location does not have parent: "); + return; + } assertUpdate("MERGE INTO " + tableName + " USING (VALUES 1) t(x) ON false" + " WHEN NOT MATCHED THEN INSERT VALUES ('str4', 4)", 1); assertQuery("SELECT * FROM " + tableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3), ('str4', 4)"); @@ -202,20 +223,29 @@ public void testOptimizeWithProvidedTableLocation(boolean partitioned, LocationP "WITH (" + locationQueryPart + partitionQueryPart + ")"); try (UncheckedCloseable ignored = onClose("DROP TABLE " + tableName)) { // create multiple data files, INSERT with multiple values would create only one file (if not partitioned) + if (locationPattern == TWO_TRAILING_SLASHES && getClass().getName().contains(".deltalake.")) { + // TODO (https://github.com/trinodb/trino/issues/17966): writes fail when Delta table is declared with location ending with two slashes + assertThatThrownBy(() -> query("INSERT INTO " + tableName + " VALUES (1, 'one')")) + .hasMessageStartingWith("Location does not have parent: "); + return; + } assertUpdate("INSERT INTO " + tableName + " VALUES (1, 'one')", 1); assertUpdate("INSERT INTO " + tableName + " VALUES (2, 'a//double_slash')", 1); assertUpdate("INSERT INTO " + tableName + " VALUES (3, 'a%percent')", 1); assertUpdate("INSERT INTO " + tableName + " VALUES (4, 'a//double_slash')", 1); + assertUpdate("INSERT INTO " + tableName + " VALUES (5, 'a///triple_slash')", 1); + assertUpdate("INSERT INTO " + tableName + " VALUES (6, 'trailing_slash/')", 1); + assertUpdate("INSERT INTO " + tableName + " VALUES (7, 'two_trailing_slashes//')", 1); assertUpdate("INSERT INTO " + tableName + " VALUES (11, 'one')", 1); Set initialFiles = getActiveFiles(tableName); - assertThat(initialFiles).hasSize(5); + assertThat(initialFiles).hasSize(8); Session session = sessionForOptimize(); computeActual(session, "ALTER TABLE " + tableName + " EXECUTE OPTIMIZE"); assertThat(query("SELECT sum(key), listagg(value, ' ') WITHIN GROUP (ORDER BY value) FROM " + tableName)) - .matches("VALUES (BIGINT '21', VARCHAR 'a%percent a//double_slash a//double_slash one one')"); + .matches("VALUES (BIGINT '39', VARCHAR 'a%percent a///triple_slash a//double_slash a//double_slash one one trailing_slash/ two_trailing_slashes//')"); Set updatedFiles = getActiveFiles(tableName); validateFilesAfterOptimize(getTableLocation(tableName), initialFiles, updatedFiles); @@ -311,7 +341,9 @@ protected enum LocationPattern { REGULAR("s3://%s/%s/regular/%s"), TRAILING_SLASH("s3://%s/%s/trailing_slash/%s/"), + TWO_TRAILING_SLASHES("s3://%s/%s/two_trailing_slashes/%s//"), DOUBLE_SLASH("s3://%s/%s//double_slash/%s"), + TRIPLE_SLASH("s3://%s/%s///triple_slash/%s"), PERCENT("s3://%s/%s/a%%percent/%s"), WHITESPACE("s3://%s/%s/a whitespace/%s"), TRAILING_WHITESPACE("s3://%s/%s/trailing_whitespace/%s "), diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java index 2b795e340f47..801ca27c38d8 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveS3AndGlueMetastoreTest.java @@ -27,6 +27,8 @@ import java.util.Set; import static io.trino.plugin.hive.BaseS3AndGlueMetastoreTest.LocationPattern.DOUBLE_SLASH; +import static io.trino.plugin.hive.BaseS3AndGlueMetastoreTest.LocationPattern.TRIPLE_SLASH; +import static io.trino.plugin.hive.BaseS3AndGlueMetastoreTest.LocationPattern.TWO_TRAILING_SLASHES; import static io.trino.plugin.hive.metastore.glue.GlueHiveMetastore.createTestingGlueHiveMetastore; import static io.trino.spi.security.SelectedRole.Type.ROLE; import static io.trino.testing.TestingNames.randomNameSuffix; @@ -122,7 +124,7 @@ public void testBasicOperationsWithProvidedTableLocation(boolean partitioned, Lo String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; - if (locationPattern == DOUBLE_SLASH) { + if (locationPattern == DOUBLE_SLASH || locationPattern == TRIPLE_SLASH || locationPattern == TWO_TRAILING_SLASHES) { assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); return; } @@ -160,7 +162,7 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L try (UncheckedCloseable ignoredDropTable = onClose("DROP TABLE " + qualifiedTableName)) { String expectedTableLocation = ((schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName) // Hive normalizes double slash - .replaceAll("(? super.testOptimizeWithProvidedTableLocation(partitioned, locationPattern)) .hasMessageStartingWith("Unsupported location that cannot be internally represented: ") .hasStackTraceContaining("SQL: CREATE TABLE test_optimize_"); @@ -205,7 +207,7 @@ public void testAnalyzeWithProvidedTableLocation(boolean partitioned, LocationPa String create = "CREATE TABLE " + tableName + "(col_str, col_int)" + "WITH (external_location = '" + location + "'" + partitionQueryPart + ") " + "AS VALUES ('str1', 1), ('str2', 2), ('str3', 3)"; - if (locationPattern == DOUBLE_SLASH) { + if (locationPattern == DOUBLE_SLASH || locationPattern == TRIPLE_SLASH || locationPattern == TWO_TRAILING_SLASHES) { assertQueryFails(create, "\\QUnsupported location that cannot be internally represented: " + location); return; } From dcf57836c5424e7d5b1843a474eb36fa5cd0e290 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 20 Jun 2023 09:12:33 +0200 Subject: [PATCH 9/9] Fix Delta writes when schema location ends with two slashes --- .../java/io/trino/plugin/deltalake/DeltaLakeMetadata.java | 3 +-- .../io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) 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 691c2ae0426c..2b13a018dc94 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 @@ -168,7 +168,6 @@ import static com.google.common.collect.Sets.difference; import static com.google.common.primitives.Ints.max; import static io.trino.filesystem.Locations.appendPath; -import static io.trino.filesystem.Locations.getParent; import static io.trino.plugin.deltalake.DataFileInfo.DataFileType.DATA; import static io.trino.plugin.deltalake.DeltaLakeAnalyzeProperties.AnalyzeMode.FULL_REFRESH; import static io.trino.plugin.deltalake.DeltaLakeAnalyzeProperties.AnalyzeMode.INCREMENTAL; @@ -1903,7 +1902,7 @@ private void checkWriteAllowed(ConnectorSession session, DeltaLakeTableHandle ta private boolean allowWrite(ConnectorSession session, DeltaLakeTableHandle tableHandle) { try { - String tableMetadataDirectory = appendPath(getParent(tableHandle.getLocation()), tableHandle.getTableName()); + String tableMetadataDirectory = getTransactionLogDir(tableHandle.getLocation()); boolean requiresOptIn = transactionLogWriterFactory.newWriter(session, tableMetadataDirectory).isUnsafe(); return !requiresOptIn || unsafeWritesEnabled; } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java index 3cc62b6acbe4..1e6b80295991 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseS3AndGlueMetastoreTest.java @@ -148,12 +148,6 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L actualTableLocation = getTableLocation(qualifiedTableName); assertThat(actualTableLocation).matches(expectedTableLocationPattern); - if (locationPattern == TWO_TRAILING_SLASHES && getClass().getName().contains(".deltalake.")) { - // TODO (https://github.com/trinodb/trino/issues/17966): writes fail when Delta table is declared within schema with location ending with two slashes - assertThatThrownBy(() -> query("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)")) - .hasMessageStartingWith("Location does not have parent: "); - return; - } assertUpdate("INSERT INTO " + qualifiedTableName + " (col_str, col_int) VALUES ('str1', 1), ('str2', 2), ('str3', 3)", 3); assertQuery("SELECT col_str, col_int FROM " + qualifiedTableName, "VALUES ('str1', 1), ('str2', 2), ('str3', 3)");