BlakeOrth commented on code in PR #8801:
URL: https://github.com/apache/arrow-rs/pull/8801#discussion_r2519206709
##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -76,16 +77,18 @@ pub(crate) fn parquet_to_arrow_schema_and_fields(
key_value_metadata: Option<&Vec<KeyValue>>,
) -> Result<(Schema, Option<ParquetField>)> {
let mut metadata =
parse_key_value_metadata(key_value_metadata).unwrap_or_default();
- let maybe_schema = metadata
+ let mut maybe_schema = metadata
.remove(super::ARROW_SCHEMA_META_KEY)
.map(|value| get_arrow_schema_from_metadata(&value))
.transpose()?;
// Add the Arrow metadata to the Parquet metadata skipping keys that
collide
- if let Some(arrow_schema) = &maybe_schema {
+ if let Some(arrow_schema) = maybe_schema.as_mut() {
arrow_schema.metadata().iter().for_each(|(k, v)| {
metadata.entry(k.clone()).or_insert_with(|| v.clone());
});
+ #[cfg(feature = "geospatial")]
+ parquet_geospatial::crs::replace_keyvalue(arrow_schema, &metadata)
Review Comment:
This is just a name change related to our previous discussion here:
- https://github.com/apache/arrow-rs/pull/8801#discussion_r2501098970
As the public API currently stands we have to do _something_ to intervene
for the case where there's a key-value CRS if we want that CRS metadata
associated with the resulting `WkbArray`. Since no writers currently support
writing this format perhaps it's moot and I think it could be pretty easily
justified that this should just be removed. On the surface it seems valuable to
me to at least be able to read this case and provide the underlying CRS meta,
but if it ends up causing confusion because the output CRS isn't the same as
the potential input that could be a pretty good justification for removing this.
--
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]
