alamb commented on code in PR #8222:
URL: https://github.com/apache/arrow-rs/pull/8222#discussion_r2300651589
##########
parquet/src/basic.rs:
##########
@@ -231,9 +231,18 @@ pub enum LogicalType {
/// A Variant value.
Variant,
/// A geospatial feature in the Well-Known Binary (WKB) format with
linear/planar edges interpolation.
- Geometry,
+ Geometry {
Review Comment:
FWIW this would be a breaking API change I think so we either have to wait
for 57 in October or make some backwards compatibility adjustments
- #7835
##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -399,9 +399,32 @@ pub fn parquet_to_arrow_field(parquet_column:
&ColumnDescriptor) -> Result<Field
}
#[cfg(feature = "arrow_canonical_extension_types")]
if let Some(logical_type) = basic_info.logical_type() {
+ use arrow_schema::extension::Geography;
+
match logical_type {
LogicalType::Uuid => ret.try_with_extension_type(Uuid)?,
LogicalType::Json => ret.try_with_extension_type(Json::default())?,
+ LogicalType::Geometry { crs } =>
ret.try_with_extension_type(Geometry::new(crs))?,
+ LogicalType::Geography { crs, algorithm } => {
+ use arrow_schema::extension::GeographyAlgorithm;
+
+ use crate::format::EdgeInterpolationAlgorithm;
+
+ let algorithm = match algorithm {
+ Some(EdgeInterpolationAlgorithm::ANDOYER) =>
Some(GeographyAlgorithm::ANDOYER),
+ Some(EdgeInterpolationAlgorithm::KARNEY) =>
Some(GeographyAlgorithm::KARNEY),
+ Some(EdgeInterpolationAlgorithm::SPHERICAL) => {
+ Some(GeographyAlgorithm::SPHERICAL)
+ }
+ Some(EdgeInterpolationAlgorithm::THOMAS) =>
Some(GeographyAlgorithm::THOMAS),
+ Some(EdgeInterpolationAlgorithm::VINCENTY) => {
+ Some(GeographyAlgorithm::VINCENTY)
+ }
+ None => None,
+ _ => None,
+ };
Review Comment:
A minor comment is that it would be nice to encapsulate the geography types
a bit more, for so, for example, this looks something more like the following
```suggestion
LogicalType::Geography { crs, algorithm } => {
let algorithm = algorithm.map(|algorithm|
EdgeInterpolationAlgorithm::from);
```
##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -399,9 +399,32 @@ pub fn parquet_to_arrow_field(parquet_column:
&ColumnDescriptor) -> Result<Field
}
#[cfg(feature = "arrow_canonical_extension_types")]
if let Some(logical_type) = basic_info.logical_type() {
+ use arrow_schema::extension::Geography;
+
match logical_type {
LogicalType::Uuid => ret.try_with_extension_type(Uuid)?,
LogicalType::Json => ret.try_with_extension_type(Json::default())?,
+ LogicalType::Geometry { crs } =>
ret.try_with_extension_type(Geometry::new(crs))?,
+ LogicalType::Geography { crs, algorithm } => {
+ use arrow_schema::extension::GeographyAlgorithm;
+
+ use crate::format::EdgeInterpolationAlgorithm;
+
+ let algorithm = match algorithm {
+ Some(EdgeInterpolationAlgorithm::ANDOYER) =>
Some(GeographyAlgorithm::ANDOYER),
+ Some(EdgeInterpolationAlgorithm::KARNEY) =>
Some(GeographyAlgorithm::KARNEY),
+ Some(EdgeInterpolationAlgorithm::SPHERICAL) => {
+ Some(GeographyAlgorithm::SPHERICAL)
+ }
+ Some(EdgeInterpolationAlgorithm::THOMAS) =>
Some(GeographyAlgorithm::THOMAS),
+ Some(EdgeInterpolationAlgorithm::VINCENTY) => {
+ Some(GeographyAlgorithm::VINCENTY)
+ }
+ None => None,
+ _ => None,
+ };
Review Comment:
The same comment applies below
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
