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 2021/08/12 21:04:15 UTC
[iceberg] branch master updated: Core: allow default data location
for ObjectStorageLocationProvider (#2845)
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 ca442d1 Core: allow default data location for ObjectStorageLocationProvider (#2845)
ca442d1 is described below
commit ca442d1ac71faf5d99cdd3d41f66b349e4de5d2b
Author: Jack Ye <yz...@amazon.com>
AuthorDate: Thu Aug 12 14:04:06 2021 -0700
Core: allow default data location for ObjectStorageLocationProvider (#2845)
* Core: allow default data location for ObjectStorageLocationProvider
* add test for fallback to folder path
* update test name
---
.../java/org/apache/iceberg/LocationProviders.java | 11 +++++----
.../org/apache/iceberg/TestLocationProvider.java | 26 ++++++++++++++++++++++
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/LocationProviders.java b/core/src/main/java/org/apache/iceberg/LocationProviders.java
index 56dce3f..33e4117 100644
--- a/core/src/main/java/org/apache/iceberg/LocationProviders.java
+++ b/core/src/main/java/org/apache/iceberg/LocationProviders.java
@@ -68,13 +68,15 @@ public class LocationProviders {
}
}
+ private static String defaultDataLocation(String tableLocation, Map<String, String> properties) {
+ return properties.getOrDefault(TableProperties.WRITE_NEW_DATA_LOCATION, String.format("%s/data", tableLocation));
+ }
+
static class DefaultLocationProvider implements LocationProvider {
private final String dataLocation;
DefaultLocationProvider(String tableLocation, Map<String, String> properties) {
- this.dataLocation = stripTrailingSlash(properties.getOrDefault(
- TableProperties.WRITE_NEW_DATA_LOCATION,
- String.format("%s/data", tableLocation)));
+ this.dataLocation = stripTrailingSlash(defaultDataLocation(tableLocation, properties));
}
@Override
@@ -96,7 +98,8 @@ public class LocationProviders {
private final String context;
ObjectStoreLocationProvider(String tableLocation, Map<String, String> properties) {
- this.storageLocation = stripTrailingSlash(properties.get(OBJECT_STORE_PATH));
+ this.storageLocation = stripTrailingSlash(properties.getOrDefault(OBJECT_STORE_PATH,
+ defaultDataLocation(tableLocation, properties)));
this.context = pathContext(tableLocation);
}
diff --git a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
index a07407f..bbbb65b 100644
--- a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
+++ b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
@@ -212,4 +212,30 @@ public class TestLocationProvider extends TableTestBase {
() -> table.locationProvider()
);
}
+
+ @Test
+ public void testObjectStorageLocationProviderPathResolution() {
+ table.updateProperties()
+ .set(TableProperties.OBJECT_STORE_ENABLED, "true")
+ .commit();
+
+ Assert.assertTrue("default data location should be used when object storage path not set",
+ table.locationProvider().newDataLocation("file").contains(table.location() + "/data"));
+
+ String folderPath = "s3://random/folder/location";
+ table.updateProperties()
+ .set(TableProperties.WRITE_NEW_DATA_LOCATION, folderPath)
+ .commit();
+
+ Assert.assertTrue("folder storage path should be used when set",
+ table.locationProvider().newDataLocation("file").contains(folderPath));
+
+ String objectPath = "s3://random/object/location";
+ table.updateProperties()
+ .set(TableProperties.OBJECT_STORE_PATH, objectPath)
+ .commit();
+
+ Assert.assertTrue("object storage path should be used when set",
+ table.locationProvider().newDataLocation("file").contains(objectPath));
+ }
}