From 0dd6f2a12b95e6f58f6547b83a71238ad67350fd Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 10 Jun 2026 18:29:03 +0000 Subject: [PATCH 1/3] Introduce FileType reference implementation --- parquet/src/basic.rs | 5 + parquet/src/schema/printer.rs | 40 ++++++ parquet/src/schema/types.rs | 227 +++++++++++++++++++++++++++++++++- 3 files changed, 271 insertions(+), 1 deletion(-) diff --git a/parquet/src/basic.rs b/parquet/src/basic.rs index b4f18f311723..e54efc29d188 100644 --- a/parquet/src/basic.rs +++ b/parquet/src/basic.rs @@ -295,6 +295,8 @@ union LogicalType { 17: (GeometryType) Geometry /// A geospatial feature in the WKB format with an explicit (non-linear/non-planar) edges interpolation. 18: (GeographyType) Geography + /// A reference to an external file. + 19: File } ); @@ -1054,6 +1056,7 @@ impl ColumnOrder { LogicalType::Variant(_) | LogicalType::Geometry(_) | LogicalType::Geography(_) + | LogicalType::File | LogicalType::_Unknown { .. } => SortOrder::UNDEFINED, }, // Fall back to converted type @@ -1242,6 +1245,7 @@ impl From> for ConvertedType { | LogicalType::Variant(_) | LogicalType::Geometry(_) | LogicalType::Geography(_) + | LogicalType::File | LogicalType::_Unknown { .. } | LogicalType::Unknown => ConvertedType::NONE, }, @@ -1341,6 +1345,7 @@ impl str::FromStr for LogicalType { )), "FLOAT16" => Ok(LogicalType::Float16), "VARIANT" => Ok(LogicalType::variant(None)), + "FILE" => Ok(LogicalType::File), "GEOMETRY" => Ok(LogicalType::geometry(None)), "GEOGRAPHY" => Ok(LogicalType::geography( None, diff --git a/parquet/src/schema/printer.rs b/parquet/src/schema/printer.rs index 31d00fc23b3b..39bfc431a17e 100644 --- a/parquet/src/schema/printer.rs +++ b/parquet/src/schema/printer.rs @@ -337,6 +337,7 @@ fn print_logical_and_converted( format!("GEOGRAPHY({algorithm})") } } + LogicalType::File => "FILE".to_string(), LogicalType::Unknown => "UNKNOWN".to_string(), LogicalType::_Unknown { field_id } => format!("_Unknown({field_id})"), }, @@ -1173,4 +1174,43 @@ mod tests { assert_print_parse_message(message); } + + #[test] + fn test_print_file_type() { + let mut s = String::new(); + { + let mut p = Printer::new(&mut s); + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let size_field = Arc::new( + Type::primitive_type_builder("size", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let file_field = Type::group_type_builder("f") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field, size_field]) + .build() + .unwrap(); + let message = Type::group_type_builder("schema") + .with_fields(vec![Arc::new(file_field)]) + .build() + .unwrap(); + p.print(&message); + } + let expected = "message schema { + REQUIRED group f (FILE) { + REQUIRED BYTE_ARRAY path (STRING); + OPTIONAL INT64 size; + } +}"; + assert_eq!(&mut s, expected); + } } diff --git a/parquet/src/schema/types.rs b/parquet/src/schema/types.rs index 1f9b8590fcf6..54f671abdb5c 100644 --- a/parquet/src/schema/types.rs +++ b/parquet/src/schema/types.rs @@ -349,7 +349,7 @@ impl<'a> PrimitiveTypeBuilder<'a> { } // Check that logical type and physical type are compatible match (logical_type, self.physical_type) { - (LogicalType::Map, _) | (LogicalType::List, _) => { + (LogicalType::Map, _) | (LogicalType::List, _) | (LogicalType::File, _) => { return Err(general_err!( "{:?} cannot be applied to a primitive type for field '{}'", logical_type, @@ -645,6 +645,9 @@ impl<'a> GroupTypeBuilder<'a> { /// Creates a new `GroupType` instance from the gathered attributes. pub fn build(self) -> Result { + if let Some(LogicalType::File) = &self.logical_type { + validate_file_type_fields(self.name, &self.fields)?; + } let mut basic_info = BasicTypeInfo { name: String::from(self.name), repetition: self.repetition, @@ -663,6 +666,39 @@ impl<'a> GroupTypeBuilder<'a> { } } +fn validate_file_type_fields(name: &str, fields: &[TypePtr]) -> Result<()> { + const VALID_OPTIONAL_FIELDS: &[&str] = &["size", "offset", "etag"]; + let mut has_path = false; + for field in fields { + let field_name = field.get_basic_info().name(); + if field_name == "path" { + let is_required = field.get_basic_info().has_repetition() + && field.get_basic_info().repetition() == Repetition::REQUIRED; + if !is_required { + return Err(general_err!( + "FILE type field 'path' must be REQUIRED in group '{}'", + name + )); + } + has_path = true; + } else if !VALID_OPTIONAL_FIELDS.contains(&field_name) { + return Err(general_err!( + "FILE type group '{}' contains unrecognized field '{}'. \ + Valid fields are: path, size, offset, etag", + name, + field_name + )); + } + } + if !has_path { + return Err(general_err!( + "FILE type group '{}' must contain required field 'path'", + name + )); + } + Ok(()) +} + /// Basic type info. This contains information such as the name of the type, /// the repetition level, the logical type and the kind of the type (group, primitive). #[derive(Clone, Debug, PartialEq, Eq)] @@ -2505,6 +2541,195 @@ mod tests { assert_eq!(result_schema, expected_schema); } + #[test] + fn test_file_logical_type_roundtrip() { + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let size_field = Arc::new( + Type::primitive_type_builder("size", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let offset_field = Arc::new( + Type::primitive_type_builder("offset", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let etag_field = Arc::new( + Type::primitive_type_builder("etag", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::OPTIONAL) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let file_group = Arc::new( + Type::group_type_builder("f") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field, size_field, offset_field, etag_field]) + .build() + .unwrap(), + ); + let schema = Arc::new( + Type::group_type_builder("example") + .with_fields(vec![file_group]) + .build() + .unwrap(), + ); + let result = roundtrip_schema(schema.clone()).unwrap(); + assert_eq!(result, schema); + assert_eq!( + result + .get_fields()[0] + .get_basic_info() + .logical_type_ref(), + Some(&LogicalType::File) + ); + } + + #[test] + fn test_file_logical_type_path_only() { + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let result = Type::group_type_builder("file_field") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field]) + .build(); + assert!(result.is_ok()); + let tp = result.unwrap(); + assert_eq!( + tp.get_basic_info().logical_type_ref(), + Some(&LogicalType::File) + ); + } + + #[test] + fn test_file_logical_type_all_fields() { + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let size_field = Arc::new( + Type::primitive_type_builder("size", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let offset_field = Arc::new( + Type::primitive_type_builder("offset", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let etag_field = Arc::new( + Type::primitive_type_builder("etag", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::OPTIONAL) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let result = Type::group_type_builder("file_field") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field, size_field, offset_field, etag_field]) + .build(); + assert!(result.is_ok()); + assert_eq!(result.unwrap().get_fields().len(), 4); + } + + #[test] + fn test_file_logical_type_requires_path_field() { + let size_field = Arc::new( + Type::primitive_type_builder("size", PhysicalType::INT64) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let result = Type::group_type_builder("missing_path") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![size_field]) + .build(); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("must contain required field 'path'")); + } + + #[test] + fn test_file_logical_type_rejects_unrecognized_field() { + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let unknown_field = Arc::new( + Type::primitive_type_builder("unknown_field", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::OPTIONAL) + .build() + .unwrap(), + ); + let result = Type::group_type_builder("bad_file") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field, unknown_field]) + .build(); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("unrecognized field")); + } + + #[test] + fn test_file_logical_type_requires_required_path_field() { + let path_field = Arc::new( + Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::OPTIONAL) + .with_logical_type(Some(LogicalType::String)) + .build() + .unwrap(), + ); + let result = Type::group_type_builder("optional_path") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![path_field]) + .build(); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("'path' must be REQUIRED")); + } + + #[test] + fn test_file_logical_type_not_allowed_on_primitive() { + let result = Type::primitive_type_builder("bad", PhysicalType::BYTE_ARRAY) + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .build(); + assert!(result.is_err()); + } + #[test] fn test_parquet_schema_from_array_rejects_negative_num_children() { let elements = vec![SchemaElement { From d29e7374caf45be04e5bbf0ceb1fbdb5696f455d Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 8 Jul 2026 01:46:49 +0000 Subject: [PATCH 2/3] address comments and update based on spec changes --- parquet/src/basic.rs | 2 +- parquet/src/schema/printer.rs | 4 +- parquet/src/schema/types.rs | 317 ++++++++++++++++++++-------------- 3 files changed, 189 insertions(+), 134 deletions(-) diff --git a/parquet/src/basic.rs b/parquet/src/basic.rs index e54efc29d188..e0fb0bda8f76 100644 --- a/parquet/src/basic.rs +++ b/parquet/src/basic.rs @@ -295,7 +295,7 @@ union LogicalType { 17: (GeometryType) Geometry /// A geospatial feature in the WKB format with an explicit (non-linear/non-planar) edges interpolation. 18: (GeographyType) Geography - /// A reference to an external file. + /// A reference to a range of bytes, stored inline, elsewhere in this file, or in an external file. 19: File } ); diff --git a/parquet/src/schema/printer.rs b/parquet/src/schema/printer.rs index c3a25d8bd7eb..ef18f5a74690 100644 --- a/parquet/src/schema/printer.rs +++ b/parquet/src/schema/printer.rs @@ -1182,7 +1182,7 @@ mod tests { let mut p = Printer::new(&mut s); let path_field = Arc::new( Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::REQUIRED) + .with_repetition(Repetition::OPTIONAL) .with_logical_type(Some(LogicalType::String)) .build() .unwrap(), @@ -1207,7 +1207,7 @@ mod tests { } let expected = "message schema { REQUIRED group f (FILE) { - REQUIRED BYTE_ARRAY path (STRING); + OPTIONAL BYTE_ARRAY path (STRING); OPTIONAL INT64 size; } }"; diff --git a/parquet/src/schema/types.rs b/parquet/src/schema/types.rs index 54f671abdb5c..00d4e1fc55bc 100644 --- a/parquet/src/schema/types.rs +++ b/parquet/src/schema/types.rs @@ -666,35 +666,92 @@ impl<'a> GroupTypeBuilder<'a> { } } +/// Validates the fields of a `FILE`-annotated group against the Parquet +/// [specification]. +/// +/// A `FILE` group annotates a reference to a range of bytes. Every field is +/// optional both in the schema and in the data: a writer may omit any field +/// from the group definition, and any field that is present must have a field +/// repetition type of `OPTIONAL`. The recognized fields (identified by name) +/// and their expected physical/logical types are: +/// +/// | Field | Physical type | Logical type | +/// |----------------|---------------|--------------| +/// | `path` | `BYTE_ARRAY` | `STRING` | +/// | `offset` | `INT64` | — | +/// | `size` | `INT64` | — | +/// | `content_type` | `BYTE_ARRAY` | `STRING` | +/// | `checksum` | `BYTE_ARRAY` | `STRING` | +/// | `inline` | `BYTE_ARRAY` | — | +/// +/// [specification]: https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file fn validate_file_type_fields(name: &str, fields: &[TypePtr]) -> Result<()> { - const VALID_OPTIONAL_FIELDS: &[&str] = &["size", "offset", "etag"]; - let mut has_path = false; + // (name, expected physical type, expected logical type) + const VALID_FIELDS: &[(&str, PhysicalType, Option)] = &[ + ("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + ("offset", PhysicalType::INT64, None), + ("size", PhysicalType::INT64, None), + ( + "content_type", + PhysicalType::BYTE_ARRAY, + Some(LogicalType::String), + ), + ( + "checksum", + PhysicalType::BYTE_ARRAY, + Some(LogicalType::String), + ), + ("inline", PhysicalType::BYTE_ARRAY, None), + ]; + for field in fields { let field_name = field.get_basic_info().name(); - if field_name == "path" { - let is_required = field.get_basic_info().has_repetition() - && field.get_basic_info().repetition() == Repetition::REQUIRED; - if !is_required { - return Err(general_err!( - "FILE type field 'path' must be REQUIRED in group '{}'", - name - )); - } - has_path = true; - } else if !VALID_OPTIONAL_FIELDS.contains(&field_name) { + let Some((_, expected_physical, expected_logical)) = + VALID_FIELDS.iter().find(|(n, _, _)| *n == field_name) + else { return Err(general_err!( "FILE type group '{}' contains unrecognized field '{}'. \ - Valid fields are: path, size, offset, etag", + Valid fields are: path, offset, size, content_type, checksum, inline", name, field_name )); + }; + + // Every field present in a FILE group must be OPTIONAL. + let is_optional = field.get_basic_info().has_repetition() + && field.get_basic_info().repetition() == Repetition::OPTIONAL; + if !is_optional { + return Err(general_err!( + "FILE type field '{}' must be OPTIONAL in group '{}'", + field_name, + name + )); + } + + // FILE fields are always primitives with a fixed physical type. + if field.is_group() { + return Err(general_err!( + "FILE type field '{}' in group '{}' must be a primitive type", + field_name, + name + )); + } + if field.get_physical_type() != *expected_physical { + return Err(general_err!( + "FILE type field '{}' in group '{}' must have physical type {:?}", + field_name, + name, + expected_physical + )); + } + if field.get_basic_info().logical_type_ref() != expected_logical.as_ref() { + return Err(general_err!( + "FILE type field '{}' in group '{}' must have logical type {:?}", + field_name, + name, + expected_logical + )); } - } - if !has_path { - return Err(general_err!( - "FILE type group '{}' must contain required field 'path'", - name - )); } Ok(()) } @@ -2541,39 +2598,44 @@ mod tests { assert_eq!(result_schema, expected_schema); } - #[test] - fn test_file_logical_type_roundtrip() { - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::REQUIRED) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); - let size_field = Arc::new( - Type::primitive_type_builder("size", PhysicalType::INT64) - .with_repetition(Repetition::OPTIONAL) - .build() - .unwrap(), - ); - let offset_field = Arc::new( - Type::primitive_type_builder("offset", PhysicalType::INT64) + /// Builds an `OPTIONAL` primitive field for use inside a `FILE` group. + fn file_field(name: &str, physical: PhysicalType, logical: Option) -> TypePtr { + Arc::new( + Type::primitive_type_builder(name, physical) .with_repetition(Repetition::OPTIONAL) + .with_logical_type(logical) .build() .unwrap(), - ); - let etag_field = Arc::new( - Type::primitive_type_builder("etag", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::OPTIONAL) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); + ) + } + + /// The full set of recognized `FILE` fields, all `OPTIONAL`, per the spec. + fn all_file_fields() -> Vec { + vec![ + file_field("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + file_field("offset", PhysicalType::INT64, None), + file_field("size", PhysicalType::INT64, None), + file_field( + "content_type", + PhysicalType::BYTE_ARRAY, + Some(LogicalType::String), + ), + file_field( + "checksum", + PhysicalType::BYTE_ARRAY, + Some(LogicalType::String), + ), + file_field("inline", PhysicalType::BYTE_ARRAY, None), + ] + } + + #[test] + fn test_file_logical_type_roundtrip() { let file_group = Arc::new( Type::group_type_builder("f") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field, size_field, offset_field, etag_field]) + .with_fields(all_file_fields()) .build() .unwrap(), ); @@ -2586,139 +2648,132 @@ mod tests { let result = roundtrip_schema(schema.clone()).unwrap(); assert_eq!(result, schema); assert_eq!( - result - .get_fields()[0] - .get_basic_info() - .logical_type_ref(), + result.get_fields()[0].get_basic_info().logical_type_ref(), Some(&LogicalType::File) ); } + #[test] + fn test_file_logical_type_all_fields() { + let result = Type::group_type_builder("file_field") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(all_file_fields()) + .build(); + assert!(result.is_ok()); + assert_eq!(result.unwrap().get_fields().len(), 6); + } + #[test] fn test_file_logical_type_path_only() { - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::REQUIRED) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); + // Every field is optional, so a group may define just `path`. let result = Type::group_type_builder("file_field") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field]) + .with_fields(vec![file_field( + "path", + PhysicalType::BYTE_ARRAY, + Some(LogicalType::String), + )]) .build(); assert!(result.is_ok()); - let tp = result.unwrap(); assert_eq!( - tp.get_basic_info().logical_type_ref(), + result.unwrap().get_basic_info().logical_type_ref(), Some(&LogicalType::File) ); } #[test] - fn test_file_logical_type_all_fields() { - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::REQUIRED) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); - let size_field = Arc::new( - Type::primitive_type_builder("size", PhysicalType::INT64) - .with_repetition(Repetition::OPTIONAL) - .build() - .unwrap(), - ); - let offset_field = Arc::new( - Type::primitive_type_builder("offset", PhysicalType::INT64) - .with_repetition(Repetition::OPTIONAL) - .build() - .unwrap(), - ); - let etag_field = Arc::new( - Type::primitive_type_builder("etag", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::OPTIONAL) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); - let result = Type::group_type_builder("file_field") + fn test_file_logical_type_inline_only() { + // An inline-only group need only define `inline`. + let result = Type::group_type_builder("inline_file") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field, size_field, offset_field, etag_field]) + .with_fields(vec![file_field("inline", PhysicalType::BYTE_ARRAY, None)]) .build(); assert!(result.is_ok()); - assert_eq!(result.unwrap().get_fields().len(), 4); } #[test] - fn test_file_logical_type_requires_path_field() { - let size_field = Arc::new( - Type::primitive_type_builder("size", PhysicalType::INT64) - .with_repetition(Repetition::OPTIONAL) - .build() - .unwrap(), - ); - let result = Type::group_type_builder("missing_path") + fn test_file_logical_type_empty_group_is_allowed() { + // No field is mandatory in the schema, so an empty group is valid. + let result = Type::group_type_builder("empty_file") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![size_field]) + .with_fields(vec![]) .build(); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("must contain required field 'path'")); + assert!(result.is_ok()); } #[test] fn test_file_logical_type_rejects_unrecognized_field() { - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::REQUIRED) - .with_logical_type(Some(LogicalType::String)) - .build() - .unwrap(), - ); - let unknown_field = Arc::new( - Type::primitive_type_builder("unknown_field", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::OPTIONAL) - .build() - .unwrap(), - ); + let unknown_field = file_field("unknown_field", PhysicalType::BYTE_ARRAY, None); let result = Type::group_type_builder("bad_file") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field, unknown_field]) + .with_fields(vec![ + file_field("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + unknown_field, + ]) .build(); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("unrecognized field")); + assert_eq!( + result.unwrap_err().to_string(), + "Parquet error: FILE type group 'bad_file' contains unrecognized field \ + 'unknown_field'. Valid fields are: path, offset, size, content_type, \ + checksum, inline" + ); } #[test] - fn test_file_logical_type_requires_required_path_field() { + fn test_file_logical_type_requires_optional_fields() { + // A REQUIRED field is no longer valid: every field must be OPTIONAL. let path_field = Arc::new( Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) - .with_repetition(Repetition::OPTIONAL) + .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::String)) .build() .unwrap(), ); - let result = Type::group_type_builder("optional_path") + let result = Type::group_type_builder("required_path") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) .with_fields(vec![path_field]) .build(); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("'path' must be REQUIRED")); + assert_eq!( + result.unwrap_err().to_string(), + "Parquet error: FILE type field 'path' must be OPTIONAL in group 'required_path'" + ); + } + + #[test] + fn test_file_logical_type_rejects_wrong_physical_type() { + // `size` must be an INT64, not a BYTE_ARRAY. + let bad_size = file_field("size", PhysicalType::BYTE_ARRAY, None); + let result = Type::group_type_builder("bad_size") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![bad_size]) + .build(); + assert_eq!( + result.unwrap_err().to_string(), + "Parquet error: FILE type field 'size' in group 'bad_size' must have physical type INT64" + ); + } + + #[test] + fn test_file_logical_type_rejects_wrong_logical_type() { + // `path` must carry the STRING logical type. + let bad_path = file_field("path", PhysicalType::BYTE_ARRAY, None); + let result = Type::group_type_builder("bad_path") + .with_repetition(Repetition::REQUIRED) + .with_logical_type(Some(LogicalType::File)) + .with_fields(vec![bad_path]) + .build(); + assert_eq!( + result.unwrap_err().to_string(), + "Parquet error: FILE type field 'path' in group 'bad_path' must have logical type \ + Some(String)" + ); } #[test] From 51226264d78eb4d3703458e15d1eb71ecadc3e47 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 28 Jul 2026 19:14:17 +0000 Subject: [PATCH 3/3] update according to latest parts of spec --- parquet/src/schema/printer.rs | 8 ++++---- parquet/src/schema/types.rs | 38 +++++++++++++++++------------------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/parquet/src/schema/printer.rs b/parquet/src/schema/printer.rs index ef18f5a74690..8ce07fb2fd93 100644 --- a/parquet/src/schema/printer.rs +++ b/parquet/src/schema/printer.rs @@ -1180,8 +1180,8 @@ mod tests { let mut s = String::new(); { let mut p = Printer::new(&mut s); - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + let uri_field = Arc::new( + Type::primitive_type_builder("uri", PhysicalType::BYTE_ARRAY) .with_repetition(Repetition::OPTIONAL) .with_logical_type(Some(LogicalType::String)) .build() @@ -1196,7 +1196,7 @@ mod tests { let file_field = Type::group_type_builder("f") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field, size_field]) + .with_fields(vec![uri_field, size_field]) .build() .unwrap(); let message = Type::group_type_builder("schema") @@ -1207,7 +1207,7 @@ mod tests { } let expected = "message schema { REQUIRED group f (FILE) { - OPTIONAL BYTE_ARRAY path (STRING); + OPTIONAL BYTE_ARRAY uri (STRING); OPTIONAL INT64 size; } }"; diff --git a/parquet/src/schema/types.rs b/parquet/src/schema/types.rs index 00d4e1fc55bc..fb4616a8377e 100644 --- a/parquet/src/schema/types.rs +++ b/parquet/src/schema/types.rs @@ -677,7 +677,7 @@ impl<'a> GroupTypeBuilder<'a> { /// /// | Field | Physical type | Logical type | /// |----------------|---------------|--------------| -/// | `path` | `BYTE_ARRAY` | `STRING` | +/// | `uri` | `BYTE_ARRAY` | `STRING` | /// | `offset` | `INT64` | — | /// | `size` | `INT64` | — | /// | `content_type` | `BYTE_ARRAY` | `STRING` | @@ -688,7 +688,7 @@ impl<'a> GroupTypeBuilder<'a> { fn validate_file_type_fields(name: &str, fields: &[TypePtr]) -> Result<()> { // (name, expected physical type, expected logical type) const VALID_FIELDS: &[(&str, PhysicalType, Option)] = &[ - ("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + ("uri", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), ("offset", PhysicalType::INT64, None), ("size", PhysicalType::INT64, None), ( @@ -711,7 +711,7 @@ fn validate_file_type_fields(name: &str, fields: &[TypePtr]) -> Result<()> { else { return Err(general_err!( "FILE type group '{}' contains unrecognized field '{}'. \ - Valid fields are: path, offset, size, content_type, checksum, inline", + Valid fields are: uri, offset, size, content_type, checksum, inline", name, field_name )); @@ -2612,7 +2612,7 @@ mod tests { /// The full set of recognized `FILE` fields, all `OPTIONAL`, per the spec. fn all_file_fields() -> Vec { vec![ - file_field("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + file_field("uri", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), file_field("offset", PhysicalType::INT64, None), file_field("size", PhysicalType::INT64, None), file_field( @@ -2665,13 +2665,13 @@ mod tests { } #[test] - fn test_file_logical_type_path_only() { - // Every field is optional, so a group may define just `path`. + fn test_file_logical_type_uri_only() { + // Every field is optional, so a group may define just `uri`. let result = Type::group_type_builder("file_field") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) .with_fields(vec![file_field( - "path", + "uri", PhysicalType::BYTE_ARRAY, Some(LogicalType::String), )]) @@ -2712,14 +2712,14 @@ mod tests { .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) .with_fields(vec![ - file_field("path", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), + file_field("uri", PhysicalType::BYTE_ARRAY, Some(LogicalType::String)), unknown_field, ]) .build(); assert_eq!( result.unwrap_err().to_string(), "Parquet error: FILE type group 'bad_file' contains unrecognized field \ - 'unknown_field'. Valid fields are: path, offset, size, content_type, \ + 'unknown_field'. Valid fields are: uri, offset, size, content_type, \ checksum, inline" ); } @@ -2727,21 +2727,21 @@ mod tests { #[test] fn test_file_logical_type_requires_optional_fields() { // A REQUIRED field is no longer valid: every field must be OPTIONAL. - let path_field = Arc::new( - Type::primitive_type_builder("path", PhysicalType::BYTE_ARRAY) + let uri_field = Arc::new( + Type::primitive_type_builder("uri", PhysicalType::BYTE_ARRAY) .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::String)) .build() .unwrap(), ); - let result = Type::group_type_builder("required_path") + let result = Type::group_type_builder("required_uri") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![path_field]) + .with_fields(vec![uri_field]) .build(); assert_eq!( result.unwrap_err().to_string(), - "Parquet error: FILE type field 'path' must be OPTIONAL in group 'required_path'" + "Parquet error: FILE type field 'uri' must be OPTIONAL in group 'required_uri'" ); } @@ -2762,16 +2762,16 @@ mod tests { #[test] fn test_file_logical_type_rejects_wrong_logical_type() { - // `path` must carry the STRING logical type. - let bad_path = file_field("path", PhysicalType::BYTE_ARRAY, None); - let result = Type::group_type_builder("bad_path") + // `uri` must carry the STRING logical type. + let bad_uri = file_field("uri", PhysicalType::BYTE_ARRAY, None); + let result = Type::group_type_builder("bad_uri") .with_repetition(Repetition::REQUIRED) .with_logical_type(Some(LogicalType::File)) - .with_fields(vec![bad_path]) + .with_fields(vec![bad_uri]) .build(); assert_eq!( result.unwrap_err().to_string(), - "Parquet error: FILE type field 'path' in group 'bad_path' must have logical type \ + "Parquet error: FILE type field 'uri' in group 'bad_uri' must have logical type \ Some(String)" ); }