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 <[email protected]>
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://[email protected]/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://[email protected]/tab1/");
+ .isEqualTo("wasbs://[email protected]/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
