You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ja...@apache.org on 2023/02/09 19:42:16 UTC
[iceberg] branch master updated: Core: TableMetadata Always Strips Trailing Slash From Location (#6777)
This is an automated email from the ASF dual-hosted git repository.
jackye pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 038f831d5b Core: TableMetadata Always Strips Trailing Slash From Location (#6777)
038f831d5b is described below
commit 038f831d5b30aea78b5903ce5329906e649fde63
Author: Russell Spitzer <ru...@GMAIL.COM>
AuthorDate: Thu Feb 9 13:42:10 2023 -0600
Core: TableMetadata Always Strips Trailing Slash From Location (#6777)
---
core/src/main/java/org/apache/iceberg/TableMetadata.java | 3 ++-
.../test/java/org/apache/iceberg/TestTableMetadata.java | 14 ++++++++++++++
.../apache/iceberg/delta/TestSnapshotDeltaLakeTable.java | 4 +++-
.../org/apache/iceberg/snowflake/SnowflakeCatalogTest.java | 8 ++++----
.../iceberg/spark/actions/TestRemoveOrphanFilesAction.java | 2 +-
.../iceberg/spark/actions/TestRemoveOrphanFilesAction.java | 2 +-
.../iceberg/spark/actions/TestRemoveOrphanFilesAction.java | 2 +-
.../iceberg/spark/actions/TestRemoveOrphanFilesAction.java | 2 +-
8 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
index afa2c7ac2d..ed148fb098 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -41,6 +41,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Maps;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.apache.iceberg.transforms.Transforms;
import org.apache.iceberg.types.TypeUtil;
+import org.apache.iceberg.util.LocationUtil;
import org.apache.iceberg.util.Pair;
import org.apache.iceberg.util.PropertyUtil;
@@ -290,7 +291,7 @@ public class TableMetadata implements Serializable {
this.metadataFileLocation = metadataFileLocation;
this.formatVersion = formatVersion;
this.uuid = uuid;
- this.location = location;
+ this.location = location != null ? LocationUtil.stripTrailingSlash(location) : null;
this.lastSequenceNumber = lastSequenceNumber;
this.lastUpdatedMillis = lastUpdatedMillis;
this.lastColumnId = lastColumnId;
diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
index e587153d78..8ab0df9eca 100644
--- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
+++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
@@ -38,6 +38,7 @@ import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -1562,6 +1563,19 @@ public class TestTableMetadata {
1));
}
+ @Test
+ public void testNoTrailingLocationSlash() {
+ String locationWithSlash = "/with_trailing_slash/";
+ String locationWithoutSlash = "/with_trailing_slash";
+ TableMetadata meta =
+ TableMetadata.newTableMetadata(
+ TEST_SCHEMA, SPEC_5, SORT_ORDER_3, locationWithSlash, Collections.emptyMap());
+ Assert.assertEquals(
+ "Metadata should never return a location ending in a slash",
+ locationWithoutSlash,
+ meta.location());
+ }
+
private String createManifestListWithManifestFile(
long snapshotId, Long parentSnapshotId, String manifestFile) throws IOException {
File manifestList = temp.newFile("manifests" + UUID.randomUUID());
diff --git a/delta-lake/src/integration/java/org/apache/iceberg/delta/TestSnapshotDeltaLakeTable.java b/delta-lake/src/integration/java/org/apache/iceberg/delta/TestSnapshotDeltaLakeTable.java
index c79129e0af..55d5106462 100644
--- a/delta-lake/src/integration/java/org/apache/iceberg/delta/TestSnapshotDeltaLakeTable.java
+++ b/delta-lake/src/integration/java/org/apache/iceberg/delta/TestSnapshotDeltaLakeTable.java
@@ -42,6 +42,7 @@ import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.spark.Spark3Util;
import org.apache.iceberg.spark.SparkCatalog;
+import org.apache.iceberg.util.LocationUtil;
import org.apache.spark.sql.Dataset;
import org.apache.spark.sql.Row;
import org.apache.spark.sql.SaveMode;
@@ -327,7 +328,8 @@ public class TestSnapshotDeltaLakeTable extends SparkDeltaLakeSnapshotTestBase {
private void checkIcebergTableLocation(String icebergTableIdentifier, String expectedLocation) {
Table icebergTable = getIcebergTable(icebergTableIdentifier);
- Assertions.assertThat(icebergTable.location()).isEqualTo(expectedLocation);
+ Assertions.assertThat(icebergTable.location())
+ .isEqualTo(LocationUtil.stripTrailingSlash(expectedLocation));
}
private void checkIcebergTableProperties(
diff --git a/snowflake/src/test/java/org/apache/iceberg/snowflake/SnowflakeCatalogTest.java b/snowflake/src/test/java/org/apache/iceberg/snowflake/SnowflakeCatalogTest.java
index 9f66f352e8..4863f55f74 100644
--- a/snowflake/src/test/java/org/apache/iceberg/snowflake/SnowflakeCatalogTest.java
+++ b/snowflake/src/test/java/org/apache/iceberg/snowflake/SnowflakeCatalogTest.java
@@ -87,7 +87,7 @@ public class SnowflakeCatalogTest {
"s3://tab1/metadata/v3.metadata.json",
TableMetadataParser.toJson(
TableMetadata.newTableMetadata(
- schema, partitionSpec, "s3://tab1/", ImmutableMap.<String, String>of()))
+ schema, partitionSpec, "s3://tab1", ImmutableMap.<String, String>of()))
.getBytes());
fakeFileIO.addFile(
"wasbs://mycontainer@myaccount.blob.core.windows.net/tab3/metadata/v334.metadata.json",
@@ -216,20 +216,20 @@ public class SnowflakeCatalogTest {
@Test
public void testLoadS3Table() {
Table table = catalog.loadTable(TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TAB_1"));
- Assertions.assertThat(table.location()).isEqualTo("s3://tab1/");
+ Assertions.assertThat(table.location()).isEqualTo("s3://tab1");
}
@Test
public void testLoadAzureTable() {
Table table = catalog.loadTable(TableIdentifier.of(Namespace.of("DB_2", "SCHEMA_2"), "TAB_3"));
Assertions.assertThat(table.location())
- .isEqualTo("wasbs://mycontainer@myaccount.blob.core.windows.net/tab1/");
+ .isEqualTo("wasbs://mycontainer@myaccount.blob.core.windows.net/tab1");
}
@Test
public void testLoadGcsTable() {
Table table = catalog.loadTable(TableIdentifier.of(Namespace.of("DB_3", "SCHEMA_3"), "TAB_5"));
- Assertions.assertThat(table.location()).isEqualTo("gs://tab5/");
+ Assertions.assertThat(table.location()).isEqualTo("gs://tab5");
}
@Test
diff --git a/spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
index 84ccb86fac..9369ca6617 100644
--- a/spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
+++ b/spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
@@ -635,7 +635,7 @@ public class TestRemoveOrphanFilesAction extends SparkTestBase {
Assert.assertTrue(
"trash file should be removed",
StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
- .anyMatch(file -> file.contains("file:" + location + "data/trashfile")));
+ .anyMatch(file -> file.contains("file:" + location + "/data/trashfile")));
}
@Test
diff --git a/spark/v3.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v3.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
index da988fddbb..8877fea51c 100644
--- a/spark/v3.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
+++ b/spark/v3.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
@@ -635,7 +635,7 @@ public abstract class TestRemoveOrphanFilesAction extends SparkTestBase {
Assert.assertTrue(
"trash file should be removed",
StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
- .anyMatch(file -> file.contains("file:" + location + "data/trashfile")));
+ .anyMatch(file -> file.contains("file:" + location + "/data/trashfile")));
}
@Test
diff --git a/spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
index c3a6aa9e94..1c38d61697 100644
--- a/spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
+++ b/spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
@@ -737,7 +737,7 @@ public abstract class TestRemoveOrphanFilesAction extends SparkTestBase {
Assert.assertTrue(
"trash file should be removed",
StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
- .anyMatch(file -> file.contains("file:" + location + "data/trashfile")));
+ .anyMatch(file -> file.contains("file:" + location + "/data/trashfile")));
}
@Test
diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
index c3a6aa9e94..1c38d61697 100644
--- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
+++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java
@@ -737,7 +737,7 @@ public abstract class TestRemoveOrphanFilesAction extends SparkTestBase {
Assert.assertTrue(
"trash file should be removed",
StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
- .anyMatch(file -> file.contains("file:" + location + "data/trashfile")));
+ .anyMatch(file -> file.contains("file:" + location + "/data/trashfile")));
}
@Test