Throw on {write.folder-storage.path,write.object-storage.path} properties

core/src/main/java/org/apache/iceberg/LocationProviders.java

@@ -20,10 +20,12 @@
 
 import java.nio.charset.StandardCharsets;
 import java.util.Map;
+import java.util.Set;
 import org.apache.hadoop.fs.Path;
 import org.apache.iceberg.common.DynConstructors;
 import org.apache.iceberg.io.LocationProvider;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
 import org.apache.iceberg.relocated.com.google.common.hash.HashCode;
 import org.apache.iceberg.relocated.com.google.common.hash.HashFunction;
 import org.apache.iceberg.relocated.com.google.common.hash.Hashing;
@@ -75,6 +77,23 @@ public static LocationProvider locationsFor(
     }
   }
 
+  private static final Set<String> DEPRECATED_PROPERTIES =
+      ImmutableSet.of(
+          TableProperties.OBJECT_STORE_PATH, TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
+
+  private static String getAndCheckLegacyLocation(Map<String, String> properties, String key) {
+    String value = properties.get(key);
+
+    if (value != null && DEPRECATED_PROPERTIES.contains(key)) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Property '%s' has been deprecated and will be removed in 2.0, use '%s' instead.",
+              key, TableProperties.WRITE_DATA_LOCATION));
+    }
+
+    return value;
+  }
+
   static class DefaultLocationProvider implements LocationProvider {
     private final String dataLocation;
 
@@ -83,9 +102,11 @@ static class DefaultLocationProvider implements LocationProvider {
     }
 
     private static String dataLocation(Map<String, String> properties, String tableLocation) {
-      String dataLocation = properties.get(TableProperties.WRITE_DATA_LOCATION);
+      String dataLocation =
+          getAndCheckLegacyLocation(properties, TableProperties.WRITE_DATA_LOCATION);
       if (dataLocation == null) {
-        dataLocation = properties.get(TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
+        dataLocation =
+            getAndCheckLegacyLocation(properties, TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
         if (dataLocation == null) {
           dataLocation = String.format("%s/data", tableLocation);
         }
@@ -135,11 +156,13 @@ static class ObjectStoreLocationProvider implements LocationProvider {
     }
 
     private static String dataLocation(Map<String, String> properties, String tableLocation) {
-      String dataLocation = properties.get(TableProperties.WRITE_DATA_LOCATION);
+      String dataLocation =
+          getAndCheckLegacyLocation(properties, TableProperties.WRITE_DATA_LOCATION);
       if (dataLocation == null) {
-        dataLocation = properties.get(TableProperties.OBJECT_STORE_PATH);
+        dataLocation = getAndCheckLegacyLocation(properties, TableProperties.OBJECT_STORE_PATH);
         if (dataLocation == null) {
-          dataLocation = properties.get(TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
+          dataLocation =
+              getAndCheckLegacyLocation(properties, TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
           if (dataLocation == null) {
             dataLocation = String.format("%s/data", tableLocation);
           }

core/src/main/java/org/apache/iceberg/TableProperties.java

@@ -250,14 +250,14 @@ private TableProperties() {}
   public static final boolean WRITE_OBJECT_STORE_PARTITIONED_PATHS_DEFAULT = true;
 
   /**
-   * @deprecated Use {@link #WRITE_DATA_LOCATION} instead.
+   * @deprecated will be removed in 2.0.0, use {@link #WRITE_DATA_LOCATION} instead.
    */
   @Deprecated public static final String OBJECT_STORE_PATH = "write.object-storage.path";
 
   public static final String WRITE_LOCATION_PROVIDER_IMPL = "write.location-provider.impl";
 
   /**
-   * @deprecated Use {@link #WRITE_DATA_LOCATION} instead.
+   * @deprecated will be removed in 2.0.0, use {@link #WRITE_DATA_LOCATION} instead.
    */
   @Deprecated
   public static final String WRITE_FOLDER_STORAGE_LOCATION = "write.folder-storage.path";

core/src/test/java/org/apache/iceberg/TestLocationProvider.java

@@ -214,61 +214,44 @@ public void testInvalidArgTypesDynamicallyLoadedLocationProvider() {
   }
 
   @TestTemplate
-  public void testObjectStorageLocationProviderPathResolution() {
-    table.updateProperties().set(TableProperties.OBJECT_STORE_ENABLED, "true").commit();
-
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("default data location should be used when object storage path not set")
-        .contains(table.location() + "/data");
-
-    String folderPath = "s3://random/folder/location";
+  public void testObjectStorageLocationProviderThrowOnDeprecatedProperties() {
+    String objectPath = "s3://random/object/location";
     table
         .updateProperties()
-        .set(TableProperties.WRITE_FOLDER_STORAGE_LOCATION, folderPath)
+        .set(TableProperties.OBJECT_STORE_ENABLED, "true")
+        .set(TableProperties.WRITE_FOLDER_STORAGE_LOCATION, objectPath)
         .commit();
 
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("folder storage path should be used when set")
-        .contains(folderPath);
-
-    String objectPath = "s3://random/object/location";
-    table.updateProperties().set(TableProperties.OBJECT_STORE_PATH, objectPath).commit();
+    assertThatThrownBy(() -> table.locationProvider().newDataLocation("file"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Property 'write.folder-storage.path' has been deprecated and will be removed in 2.0, use 'write.data.path' instead.");
 
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("object storage path should be used when set")
-        .contains(objectPath);
+    table
+        .updateProperties()
+        .set(TableProperties.OBJECT_STORE_PATH, objectPath)
+        .remove(TableProperties.WRITE_FOLDER_STORAGE_LOCATION)
+        .commit();
 
-    String dataPath = "s3://random/data/location";
-    table.updateProperties().set(TableProperties.WRITE_DATA_LOCATION, dataPath).commit();
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("write data path should be used when set")
-        .contains(dataPath);
+    assertThatThrownBy(() -> table.locationProvider().newDataLocation("file"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Property 'write.object-storage.path' has been deprecated and will be removed in 2.0, use 'write.data.path' instead.");
   }
 
   @TestTemplate
-  public void testDefaultStorageLocationProviderPathResolution() {
-    table.updateProperties().set(TableProperties.OBJECT_STORE_ENABLED, "false").commit();
-
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("default data location should be used when object storage path not set")
-        .contains(table.location() + "/data");
-
+  public void testDefaultStorageLocationProviderThrowOnDeprecatedProperties() {
     String folderPath = "s3://random/folder/location";
     table
         .updateProperties()
+        .set(TableProperties.OBJECT_STORE_ENABLED, "false")
         .set(TableProperties.WRITE_FOLDER_STORAGE_LOCATION, folderPath)
         .commit();
 
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("folder storage path should be used when set")
-        .contains(folderPath);
-
-    String dataPath = "s3://random/data/location";
-    table.updateProperties().set(TableProperties.WRITE_DATA_LOCATION, dataPath).commit();
-
-    assertThat(table.locationProvider().newDataLocation("file"))
-        .as("write data path should be used when set")
-        .contains(dataPath);
+    assertThatThrownBy(() -> table.locationProvider().newDataLocation("file"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Property 'write.folder-storage.path' has been deprecated and will be removed in 2.0, use 'write.data.path' instead.");
   }
 
   @TestTemplate

docs/docs/aws.md

@@ -385,6 +385,7 @@ Note, the path resolution logic for `ObjectStoreLocationProvider` is `write.data
 However, for the older versions up to 0.12.0, the logic is as follows:
 - before 0.12.0, `write.object-storage.path` must be set.
 - at 0.12.0, `write.object-storage.path` then `write.folder-storage.path` then `<tableLocation>/data`.
+- at 2.0.0 `write.object-storage.path` and `write.folder-storage.path` will be removed
 
 For more details, please refer to the [LocationProvider Configuration](custom-catalog.md#custom-location-provider-implementation) section.