This is an automated email from the ASF dual-hosted git repository.
CTTY pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
The following commit(s) were added to refs/heads/main by this push:
new 8adaa872f feat(writer): URL encode field names for partition paths
(#2875)
8adaa872f is described below
commit 8adaa872f31549dd5ad8255848715758228038bc
Author: hsiang-c <[email protected]>
AuthorDate: Thu Jul 30 14:45:55 2026 -0700
feat(writer): URL encode field names for partition paths (#2875)
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->
- Closes #2874
## What changes are included in this PR?
<!--
Provide a summary of the modifications in this PR. List the main changes
such as new features, bug fixes, refactoring, or any other updates.
-->
- URL encoded the string components used in the `partition_to_path`
function so that we can tolerate schema with special characters in the
fields.
## Are these changes tested?
<!--
Specify what test covers (unit test, integration test, etc.).
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Unit tests
---------
Co-authored-by: Shawn Chang <[email protected]>
---
Cargo.lock | 1 +
Cargo.toml | 1 +
crates/iceberg/Cargo.toml | 1 +
crates/iceberg/src/spec/partition.rs | 73 +++++++++++++++++++---
.../src/writer/file_writer/location_generator.rs | 58 +++++++++++++----
5 files changed, 115 insertions(+), 19 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index b2269dc51..d7660914a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3778,6 +3778,7 @@ dependencies = [
"fastnum",
"flate2",
"fnv",
+ "form_urlencoded",
"futures",
"iceberg_test_utils",
"itertools 0.13.0",
diff --git a/Cargo.toml b/Cargo.toml
index a394ec107..693c9ffc9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -93,6 +93,7 @@ fastnum = { version = "0.7", default-features = false,
features = [
faststr = "0.2.31"
flate2 = "1.1.5"
fnv = "1.0.7"
+form_urlencoded = "1.2.2"
fs-err = "3.1.0"
futures = "0.3"
hive_metastore = "0.2.0"
diff --git a/crates/iceberg/Cargo.toml b/crates/iceberg/Cargo.toml
index c33445c64..66eadfb7e 100644
--- a/crates/iceberg/Cargo.toml
+++ b/crates/iceberg/Cargo.toml
@@ -57,6 +57,7 @@ expect-test = { workspace = true }
fastnum = { workspace = true }
flate2 = { workspace = true }
fnv = { workspace = true }
+form_urlencoded = { workspace = true }
futures = { workspace = true }
itertools = { workspace = true }
moka = { version = "0.12.10", features = ["future"] }
diff --git a/crates/iceberg/src/spec/partition.rs
b/crates/iceberg/src/spec/partition.rs
index 311077f3c..01c69080d 100644
--- a/crates/iceberg/src/spec/partition.rs
+++ b/crates/iceberg/src/spec/partition.rs
@@ -166,13 +166,14 @@ impl PartitionSpec {
.enumerate()
.map(|(i, field)| {
let value = data[i].as_ref();
- format!(
- "{}={}",
- field.name,
- field
- .transform
- .to_human_string(&field_types[i].field_type, value)
- )
+ form_urlencoded::Serializer::new(String::new())
+ .append_pair(
+ &field.name,
+ &field
+ .transform
+ .to_human_string(&field_types[i].field_type,
value),
+ )
+ .finish()
})
.join("/")
}
@@ -1750,4 +1751,62 @@ mod tests {
"id=42/name=alice/ts_hour=1000/empty_void=null"
);
}
+
+ #[test]
+ fn test_partition_to_path_escaped_strings() {
+ let schema = Schema::builder()
+ .with_fields(vec![
+ NestedField::required(1, "\"esc\"#1",
Type::Primitive(PrimitiveType::String))
+ .into(),
+ NestedField::required(2, "data",
Type::Primitive(PrimitiveType::String)).into(),
+ ])
+ .build()
+ .unwrap();
+
+ let spec = PartitionSpec::builder(schema.clone())
+ .add_partition_field("\"esc\"#1", "\"esc\"#1", Transform::Identity)
+ .unwrap()
+ .build()
+ .unwrap();
+
+ let data = Struct::from_iter([
+ Some(Literal::string("a/b/c/d")),
+ Some(Literal::string("val#1")),
+ ]);
+
+ assert_eq!(
+ spec.partition_to_path(&data, schema.into()),
+ "%22esc%22%231=a%2Fb%2Fc%2Fd"
+ );
+ }
+
+ #[test]
+ fn test_partition_to_path_escaped_field_name() {
+ let schema = Schema::builder()
+ .with_fields(vec![
+ NestedField::required(1, "\"esc\"#1",
Type::Primitive(PrimitiveType::String))
+ .into(),
+ NestedField::required(2, "data",
Type::Primitive(PrimitiveType::String)).into(),
+ ])
+ .build()
+ .unwrap();
+
+ let spec = PartitionSpec::builder(schema.clone())
+ .add_partition_field("data", "data", Transform::Identity)
+ .unwrap()
+ .add_partition_field("data", "data_truc_10",
Transform::Truncate(10))
+ .unwrap()
+ .build()
+ .unwrap();
+
+ let data = Struct::from_iter([
+ Some(Literal::string("a/b/c/d")),
+ Some(Literal::string("a/b/c/d")),
+ ]);
+
+ assert_eq!(
+ spec.partition_to_path(&data, schema.into()),
+ "data=a%2Fb%2Fc%2Fd/data_truc_10=a%2Fb%2Fc%2Fd"
+ );
+ }
}
diff --git a/crates/iceberg/src/writer/file_writer/location_generator.rs
b/crates/iceberg/src/writer/file_writer/location_generator.rs
index 6b5b705f0..d37d00c93 100644
--- a/crates/iceberg/src/writer/file_writer/location_generator.rs
+++ b/crates/iceberg/src/writer/file_writer/location_generator.rs
@@ -270,10 +270,52 @@ pub(crate) mod test {
assert_eq!(location, "/base/path/id=42/name=alice/data-00000.parquet");
// Create a table metadata for DefaultLocationGenerator
- let table_metadata = TableMetadata {
+ let table_metadata = table_metadata_with("s3://data.db/table",
HashMap::new());
+
+ // Test with DefaultLocationGenerator
+ let default_location_gen =
DefaultLocationGenerator::new(&table_metadata).unwrap();
+ let location =
default_location_gen.generate_location(Some(&partition_key), file_name);
+ assert_eq!(
+ location,
+ "s3://data.db/table/data/id=42/name=alice/data-00000.parquet"
+ );
+ }
+
+ #[test]
+ fn test_location_generate_with_special_characters_partition() {
+ let schema = Arc::new(
+ Schema::builder()
+ .with_schema_id(1)
+ .with_fields(vec![
+ NestedField::required(1, "data#1",
Type::Primitive(PrimitiveType::Int)).into(),
+ ])
+ .build()
+ .unwrap(),
+ );
+ let partition_spec = PartitionSpec::builder(schema.clone())
+ .add_partition_field("data#1", "data#1", Transform::Identity)
+ .unwrap()
+ .build()
+ .unwrap();
+ let partition_data =
Struct::from_iter([Some(Literal::string("val#1"))]);
+ let partition_key = PartitionKey::new(partition_spec, schema,
partition_data);
+
+ let table_metadata = table_metadata_with("s3://data.db/table",
HashMap::new());
+ let location_gen =
DefaultLocationGenerator::new(&table_metadata).unwrap();
+ let location = location_gen.generate_location(Some(&partition_key),
"test.parquet");
+
+ assert_eq!(
+ location,
+ "s3://data.db/table/data/data%231=val%231/test.parquet"
+ );
+ }
+
+ /// Build a minimal `TableMetadata` for location generator tests.
+ fn table_metadata_with(location: &str, properties: HashMap<String,
String>) -> TableMetadata {
+ TableMetadata {
format_version: FormatVersion::V2,
table_uuid:
Uuid::parse_str("fb072c92-a02b-11e9-ae9c-1bb7bc9eca94").unwrap(),
- location: "s3://data.db/table".to_string(),
+ location: location.to_string(),
last_updated_ms: 1515100955770,
last_column_id: 2,
schemas: HashMap::new(),
@@ -287,7 +329,7 @@ pub(crate) mod test {
snapshots: HashMap::default(),
current_snapshot_id: None,
last_sequence_number: 1,
- properties: HashMap::new(),
+ properties,
snapshot_log: Vec::new(),
metadata_log: vec![],
refs: HashMap::new(),
@@ -295,14 +337,6 @@ pub(crate) mod test {
partition_statistics: HashMap::new(),
encryption_keys: HashMap::new(),
next_row_id: 0,
- };
-
- // Test with DefaultLocationGenerator
- let default_location_gen =
DefaultLocationGenerator::new(&table_metadata).unwrap();
- let location =
default_location_gen.generate_location(Some(&partition_key), file_name);
- assert_eq!(
- location,
- "s3://data.db/table/data/id=42/name=alice/data-00000.parquet"
- );
+ }
}
}
