[iceberg] branch master updated: Core: Skip context for object storage in table prefixes (#4349)

[blue]

This is an automated email from the ASF dual-hosted git repository.

blue 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 30d62f9  Core: Skip context for object storage in table prefixes 
(#4349)
30d62f9 is described below

commit 30d62f9ab04b1ce973937509eeeb43e950936653
Author: Ryan Blue <b...@apache.org>
AuthorDate: Wed Mar 16 16:18:06 2022 -0700

    Core: Skip context for object storage in table prefixes (#4349)
---
 .../java/org/apache/iceberg/LocationProviders.java    | 13 +++++++++++--
 .../java/org/apache/iceberg/TestLocationProvider.java | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/LocationProviders.java 
b/core/src/main/java/org/apache/iceberg/LocationProviders.java
index f735d9b..622dd9d 100644
--- a/core/src/main/java/org/apache/iceberg/LocationProviders.java
+++ b/core/src/main/java/org/apache/iceberg/LocationProviders.java
@@ -104,7 +104,12 @@ public class LocationProviders {
 
     ObjectStoreLocationProvider(String tableLocation, Map<String, String> 
properties) {
       this.storageLocation = stripTrailingSlash(dataLocation(properties, 
tableLocation));
-      this.context = pathContext(tableLocation);
+      // if the storage location is within the table prefix, don't add table 
and database name context
+      if (storageLocation.startsWith(tableLocation)) {
+        this.context = null;
+      } else {
+        this.context = pathContext(tableLocation);
+      }
     }
 
     private static String dataLocation(Map<String, String> properties, String 
tableLocation) {
@@ -129,7 +134,11 @@ public class LocationProviders {
     @Override
     public String newDataLocation(String filename) {
       int hash = HASH_FUNC.apply(filename);
-      return String.format("%s/%08x/%s/%s", storageLocation, hash, context, 
filename);
+      if (context != null) {
+        return String.format("%s/%08x/%s/%s", storageLocation, hash, context, 
filename);
+      } else {
+        return String.format("%s/%08x/%s", storageLocation, hash, filename);
+      }
     }
 
     private static String pathContext(String tableLocation) {
diff --git a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java 
b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
index c20a4df..bdb8d6c 100644
--- a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
+++ b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java
@@ -19,8 +19,10 @@
 
 package org.apache.iceberg;
 
+import java.util.List;
 import java.util.Map;
 import org.apache.iceberg.io.LocationProvider;
+import org.apache.iceberg.relocated.com.google.common.base.Splitter;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -272,4 +274,21 @@ public class TestLocationProvider extends TableTestBase {
     Assert.assertTrue("write data path should be used when set",
         table.locationProvider().newDataLocation("file").contains(dataPath));
   }
+
+  @Test
+  public void testObjectStorageWithinTableLocation() {
+    table.updateProperties()
+        .set(TableProperties.OBJECT_STORE_ENABLED, "true")
+        .commit();
+
+    String fileLocation = 
table.locationProvider().newDataLocation("test.parquet");
+    String relativeLocation = fileLocation.replaceFirst(table.location(), "");
+    List<String> parts = Splitter.on("/").splitToList(relativeLocation);
+
+    Assert.assertEquals("Should contain 4 parts", 4, parts.size());
+    Assert.assertTrue("First part should be empty", parts.get(0).isEmpty());
+    Assert.assertEquals("Second part should be data", "data", parts.get(1));
+    Assert.assertFalse("Third part should be a hash value", 
parts.get(2).isEmpty());
+    Assert.assertEquals("Fourth part should be the file name passed in", 
"test.parquet", parts.get(3));
+  }
 }