etseidl commented on code in PR #9997:
URL: https://github.com/apache/arrow-rs/pull/9997#discussion_r3277438374
##########
parquet/src/file/metadata/thrift/mod.rs:
##########
@@ -621,6 +621,26 @@ fn read_column_chunk<'a>(
Ok(col)
}
+fn read_schema(prot: &mut ThriftSliceInputProtocol) ->
Result<SchemaDescriptor> {
+ let mut schema = read_thrift_vec::<SchemaElement,
ThriftSliceInputProtocol>(&mut *prot)?;
+ // An earlier version of this crate enforced this when decoding
LogicalType. Now that
+ // the decoder is macro generated, we do this to preserve the original
behavior.
+ // TODO: this was done due to a line in the spec saying an unset algorithm
defaults
+ // to SPHERICAL. But there is no default in the Thrift, so it would be
better to set the
+ // default when consumed.
+ // See
https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#geography
+ for se in schema.iter_mut() {
+ match se.logical_type.as_mut() {
+ Some(LogicalType::Geography(g)) if g.algorithm.is_none() => {
+ g.algorithm = Some(Default::default());
+ }
Review Comment:
Awesome, thanks for the quick response! I'll go ahead and remove this then
and add the `algorithm()` function I mentioned. I'll also update the API docs
for the struct to make clear that if the field is `None`, `SPHERICAL` should be
used.
> A heads up that there's a bug in that extension type (I will fix but
haven't gotten there yet): https://github.com/apache/arrow-rs/issues/9929
👍 Thanks for the heads up!
--
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]
