Merge branch 'main' into field_id_nested_types

Author: mbutrovich

crates/catalog/hms/tests/hms_catalog_test.rs

@@ -23,7 +23,10 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use iceberg::io::{FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY};
+use iceberg::io::{
+    FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_PATH_STYLE_ACCESS, S3_REGION,
+    S3_SECRET_ACCESS_KEY,
+};
 use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent};
 use iceberg_catalog_hms::{
     HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE,
@@ -56,6 +59,7 @@ async fn get_catalog() -> HmsCatalog {
         (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()),
         (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()),
         (S3_REGION.to_string(), "us-east-1".to_string()),
+        (S3_PATH_STYLE_ACCESS.to_string(), "true".to_string()),
     ]);
 
     // Wait for bucket to actually exist

crates/catalog/loader/tests/common/mod.rs

@@ -24,8 +24,8 @@ use std::fmt;
 use std::sync::Arc;
 
 use iceberg::io::{
-    FileIOBuilder, LocalFsStorageFactory, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION,
-    S3_SECRET_ACCESS_KEY,
+    FileIOBuilder, LocalFsStorageFactory, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_PATH_STYLE_ACCESS,
+    S3_REGION, S3_SECRET_ACCESS_KEY,
 };
 use iceberg::memory::{MEMORY_CATALOG_WAREHOUSE, MemoryCatalogBuilder};
 use iceberg::spec::{NestedField, PrimitiveType, Schema, Type};
@@ -229,6 +229,7 @@ async fn glue_catalog() -> GlueCatalog {
         (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()),
         (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()),
         (S3_REGION.to_string(), "us-east-1".to_string()),
+        (S3_PATH_STYLE_ACCESS.to_string(), "true".to_string()),
     ]);
 
     let file_io = FileIOBuilder::new(Arc::new(OpenDalStorageFactory::S3 {
@@ -280,6 +281,7 @@ async fn hms_catalog() -> HmsCatalog {
         (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()),
         (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()),
         (S3_REGION.to_string(), "us-east-1".to_string()),
+        (S3_PATH_STYLE_ACCESS.to_string(), "true".to_string()),
     ]);
 
     let file_io = FileIOBuilder::new(Arc::new(OpenDalStorageFactory::S3 {

crates/catalog/s3tables/src/catalog.rs

@@ -202,7 +202,7 @@ impl S3TablesCatalog {
         // Use provided factory or default to OpenDalStorageFactory::S3
         let factory = storage_factory.unwrap_or_else(|| {
             Arc::new(OpenDalStorageFactory::S3 {
-                configured_scheme: "s3a".to_string(),
+                configured_scheme: "s3".to_string(),
                 customized_credential_load: None,
             })
         });

crates/iceberg/src/io/storage/config/s3.rs

@@ -69,8 +69,14 @@ pub const S3_DISABLE_CONFIG_LOAD: &str = "s3.disable-config-load";
 ///
 /// This struct contains all the configuration options for connecting to Amazon S3.
 /// Use the builder pattern via `S3Config::builder()` to construct instances.
-/// ```
-#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, TypedBuilder)]
+///
+/// Defaults follow the Iceberg `S3FileIOProperties` spec (see
+/// [`PATH_STYLE_ACCESS_DEFAULT = false`](https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIOProperties.java)),
+/// i.e. virtual-host-style addressing is enabled unless
+/// `s3.path-style-access=true` is explicitly set. This matches what
+/// Java clients do out of the box and is required for a number of
+/// S3-compatible stores that do not support path-style URLs.
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, TypedBuilder)]
 pub struct S3Config {
     /// S3 endpoint URL.
     #[builder(default, setter(strip_option, into))]
@@ -88,7 +94,9 @@ pub struct S3Config {
     #[builder(default, setter(strip_option, into))]
     pub region: Option<String>,
     /// Enable virtual host style (opposite of path style access).
-    #[builder(default)]
+    ///
+    /// Defaults to `true` to match Iceberg `S3FileIOProperties.PATH_STYLE_ACCESS_DEFAULT = false`.
+    #[builder(default = true)]
     pub enable_virtual_host_style: bool,
     /// Server side encryption type.
     #[builder(default, setter(strip_option, into))]
@@ -125,6 +133,12 @@ pub struct S3Config {
     pub disable_config_load: bool,
 }
 
+impl Default for S3Config {
+    fn default() -> Self {
+        Self::builder().build()
+    }
+}
+
 impl TryFrom<&StorageConfig> for S3Config {
     type Error = crate::Error;
 
@@ -267,6 +281,17 @@ mod tests {
         assert_eq!(s3_config.region.as_deref(), Some("eu-west-1"));
     }
 
+    #[test]
+    fn test_s3_config_default_is_virtual_host_style() {
+        // Matches Iceberg S3FileIOProperties.PATH_STYLE_ACCESS_DEFAULT = false.
+        assert!(S3Config::default().enable_virtual_host_style);
+        assert!(
+            S3Config::try_from(&StorageConfig::new())
+                .unwrap()
+                .enable_virtual_host_style
+        );
+    }
+
     #[test]
     fn test_s3_config_path_style_access() {
         let storage_config = StorageConfig::new().with_prop(S3_PATH_STYLE_ACCESS, "true");

crates/integration_tests/src/lib.rs

@@ -18,7 +18,9 @@
 use std::collections::HashMap;
 use std::sync::OnceLock;
 
-use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY};
+use iceberg::io::{
+    S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_PATH_STYLE_ACCESS, S3_REGION, S3_SECRET_ACCESS_KEY,
+};
 use iceberg_catalog_rest::REST_CATALOG_PROP_URI;
 use iceberg_test_utils::{get_minio_endpoint, get_rest_catalog_endpoint, set_up};
 
@@ -45,6 +47,7 @@ impl GlobalTestFixture {
             (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()),
             (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()),
             (S3_REGION.to_string(), "us-east-1".to_string()),
+            (S3_PATH_STYLE_ACCESS.to_string(), "true".to_string()),
         ]);
 
         GlobalTestFixture { catalog_config }

crates/storage/opendal/src/s3.rs

@@ -37,6 +37,12 @@ use crate::utils::{from_opendal_error, is_truthy};
 /// Parse iceberg props to s3 config.
 pub(crate) fn s3_config_parse(mut m: HashMap<String, String>) -> Result<S3Config> {
     let mut cfg = S3Config::default();
+    // Match Iceberg `S3FileIOProperties.PATH_STYLE_ACCESS_DEFAULT = false`:
+    // virtual-host-style addressing is the spec default. opendal's own
+    // default is path-style, which disagrees with the Java SDK and breaks
+    // S3-compatible stores that only accept virtual-hosted-style URLs.
+    // Any explicit `s3.path-style-access` property below overrides this.
+    cfg.enable_virtual_host_style = true;
     if let Some(endpoint) = m.remove(S3_ENDPOINT) {
         cfg.endpoint = Some(endpoint);
     };
@@ -177,3 +183,28 @@ impl AwsCredentialLoad for CustomAwsCredentialLoader {
         self.0.load_credential(client).await
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashMap;
+
+    use iceberg::io::S3_PATH_STYLE_ACCESS;
+
+    use super::s3_config_parse;
+
+    fn parse_with(prop: Option<&str>) -> bool {
+        let mut props = HashMap::new();
+        if let Some(v) = prop {
+            props.insert(S3_PATH_STYLE_ACCESS.to_string(), v.to_string());
+        }
+        s3_config_parse(props).unwrap().enable_virtual_host_style
+    }
+
+    #[test]
+    fn s3_config_parse_path_style_access() {
+        // Match Iceberg S3FileIOProperties.PATH_STYLE_ACCESS_DEFAULT = false.
+        assert!(parse_with(None));
+        assert!(parse_with(Some("false")));
+        assert!(!parse_with(Some("true")));
+    }
+}

crates/storage/opendal/tests/file_io_s3_test.rs

@@ -26,7 +26,8 @@ mod tests {
     use async_trait::async_trait;
     use futures::StreamExt;
     use iceberg::io::{
-        FileIO, FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY,
+        FileIO, FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_PATH_STYLE_ACCESS, S3_REGION,
+        S3_SECRET_ACCESS_KEY,
     };
     use iceberg_storage_opendal::{CustomAwsCredentialLoader, OpenDalStorageFactory};
     use iceberg_test_utils::{get_minio_endpoint, normalize_test_name_with_parts, set_up};
@@ -47,6 +48,7 @@ mod tests {
             (S3_ACCESS_KEY_ID, "admin".to_string()),
             (S3_SECRET_ACCESS_KEY, "password".to_string()),
             (S3_REGION, "us-east-1".to_string()),
+            (S3_PATH_STYLE_ACCESS, "true".to_string()),
         ])
         .build()
     }
@@ -139,6 +141,7 @@ mod tests {
             (S3_ENDPOINT, "http://localhost:9000".to_string()),
             ("bucket", "test-bucket".to_string()),
             (S3_REGION, "us-east-1".to_string()),
+            (S3_PATH_STYLE_ACCESS, "true".to_string()),
         ]);
     }
 
@@ -160,6 +163,7 @@ mod tests {
         .with_props(vec![
             (S3_ENDPOINT, minio_endpoint),
             (S3_REGION, "us-east-1".to_string()),
+            (S3_PATH_STYLE_ACCESS, "true".to_string()),
         ])
         .build();
 
@@ -188,6 +192,7 @@ mod tests {
         .with_props(vec![
             (S3_ENDPOINT, minio_endpoint),
             (S3_REGION, "us-east-1".to_string()),
+            (S3_PATH_STYLE_ACCESS, "true".to_string()),
         ])
         .build();
 

crates/storage/opendal/tests/resolving_storage_test.rs

@@ -29,7 +29,8 @@ mod tests {
     use std::sync::Arc;
 
     use iceberg::io::{
-        FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY,
+        FileIOBuilder, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_PATH_STYLE_ACCESS, S3_REGION,
+        S3_SECRET_ACCESS_KEY,
     };
     use iceberg_storage_opendal::OpenDalResolvingStorageFactory;
     use iceberg_test_utils::{get_minio_endpoint, normalize_test_name_with_parts, set_up};
@@ -45,6 +46,7 @@ mod tests {
                 (S3_ACCESS_KEY_ID, "admin".to_string()),
                 (S3_SECRET_ACCESS_KEY, "password".to_string()),
                 (S3_REGION, "us-east-1".to_string()),
+                (S3_PATH_STYLE_ACCESS, "true".to_string()),
             ])
             .build()
     }
@@ -288,6 +290,7 @@ mod tests {
             .with_props(vec![
                 (S3_ENDPOINT, minio_endpoint),
                 (S3_REGION, "us-east-1".to_string()),
+                (S3_PATH_STYLE_ACCESS, "true".to_string()),
             ])
             .build();