alamb commented on code in PR #8409:
URL: https://github.com/apache/arrow-rs/pull/8409#discussion_r2370243795
##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -390,31 +390,27 @@ pub fn parquet_to_arrow_field(parquet_column:
&ColumnDescriptor) -> Result<Field
let field = complex::convert_type(&parquet_column.self_type_ptr())?;
let mut ret = Field::new(parquet_column.name(), field.arrow_type,
field.nullable);
- let basic_info = parquet_column.self_type().get_basic_info();
- let mut meta = HashMap::with_capacity(if cfg!(feature =
"arrow_canonical_extension_types") {
- 2
- } else {
- 1
- });
+ let parquet_type = parquet_column.self_type();
+ let basic_info = parquet_type.get_basic_info();
+
+ let mut hash_map_size = 0;
if basic_info.has_id() {
- meta.insert(
+ hash_map_size += 1;
+ }
+ if has_extension_type(parquet_type) {
+ hash_map_size += 1;
+ }
+ if hash_map_size == 0 {
+ return Ok(ret);
+ }
+ ret.set_metadata(HashMap::with_capacity(hash_map_size));
+ if basic_info.has_id() {
+ ret.metadata_mut().insert(
PARQUET_FIELD_ID_META_KEY.to_string(),
basic_info.id().to_string(),
);
}
- #[cfg(feature = "arrow_canonical_extension_types")]
- if let Some(logical_type) = basic_info.logical_type() {
- match logical_type {
- LogicalType::Uuid => ret.try_with_extension_type(Uuid)?,
- LogicalType::Json => ret.try_with_extension_type(Json::default())?,
- _ => {}
- }
- }
- if !meta.is_empty() {
Review Comment:
This is the actual fix for https://github.com/apache/arrow-rs/issues/7063
The existing code has a very subtle bug -- by calling `ret.set_metadata`
here it wipes out any metadata that was attached to `ret` that was added by
try_with_extension_type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
