You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kaushiksrini (via GitHub)" <gi...@apache.org> on 2025/08/26 01:58:02 UTC

[PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

kaushiksrini opened a new pull request, #8225:
URL: https://github.com/apache/arrow-rs/pull/8225

   # Which issue does this PR close?
   
   We generally require a GitHub issue to be filed for all bug fixes and enhancements, and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax.
   
   - Builds on #7799 by supporting the new struct `GeospatialStatistics`.
   
   # Rationale for this change
   
   Why are you proposing this change? If this is already explained clearly in the issue, then this section is not needed.
   Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
   
   * This is part of a draft to support geospatial types (geometry and geography) in Parquet. This has been 
   
   # What changes are included in this PR?
   
   There is no need to duplicate the description in the issue here, but it is sometimes worth providing a summary of the individual changes in this PR.
   
   - Structs for supporting geospatial statistics information (bbox and geospatial types) derived from thrift classes. 
   - Would appreciate feedback on structure and where certain parts should go.
   
   # Are these changes tested?
   
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   
   Not as much as I would like at the moment. This is a draft to get early feedback. Will add more thorough in subsequent changes.
   
   # Are there any user-facing changes?
   
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   
   If there are any breaking changes to public APIs, please call them out.
   


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3315985015

   > Would a reasonable next step be to add a test reading one of the example files in parquet-testing/data/geospatial? It looks like there is a function that can get you the path to that directory here:
   
   Here is an example of what we did for variant: https://github.com/apache/arrow-rs/blob/main/parquet/tests/variant_integration.rs


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2299702474


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -178,15 +197,26 @@ pub fn from_thrift(geo_statistics: Option<TGeospatialStatistics>) -> Result<Opti
     Ok(match geo_statistics {
         Some(geo_stats) => {
             let bbox = if let Some(bbox) = geo_stats.bbox {
-                Some(BoundingBox::new(
-                    bbox.xmin.into(), 
-                    bbox.ymin.into(), 
-                    bbox.xmax.into(), 
-                    bbox.ymax.into(), 
-                if let Some(zmin) = bbox.zmin { Some(zmin.into()) } else { None },
-                if let Some(zmax) = bbox.zmax { Some(zmax.into()) } else { None },
-                if let Some(mmin) = bbox.mmin { Some(mmin.into()) } else { None },
-                if let Some(mmax) = bbox.mmax { Some(mmax.into()) } else { None }))
+                let mut new_bbox = BoundingBox::new(
+                    bbox.xmin.into(),
+                    bbox.ymin.into(),
+                    bbox.xmax.into(),
+                    bbox.ymax.into(),
+                );
+
+                if bbox.zmin.is_some() && bbox.zmax.is_some() {
+                    new_bbox = new_bbox.with_zrange(bbox.zmin.unwrap().into(), bbox.zmax.unwrap().into());
+                } else if bbox.zmin.is_some() != bbox.zmax.is_some() {
+                    return Err(ParquetError::General(format!("Z-coordinate values mismatch: {:?} and {:?}", bbox.zmin, bbox.zmax)));
+                }

Review Comment:
   Up to you but maybe slightly easier to read if you `match (bbox.zmin, bbox.zmax)` instead of having these two `if` clauses.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2366224349


##########
parquet/src/geospatial/mod.rs:
##########
@@ -0,0 +1,49 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! This module provides functionality for working with geospatial data in Parquet file as defied in the [spec][parquet-geo-spec].
+//!
+//! * [`GeospatialStatistics`]: describes the geospatial statistics for a Parquet column.
+//! * [`BoundingBox`]: describes the bounding box values for a geospatial column.
+//!
+//! [`GeospatialStatistics`] describes the geospatial statistics for a Parquet column.
+//! * bbox: the [`BoundingBox`] for the geospatial data
+//! * geospatial_types: the geospatial types for the geospatial data as specified in [specification][geo-types].
+//!
+//! Geospatial bounding box describes the spatial extent of the geospatial data within a Parquet row group.
+//! * xmin, xmax: the minimum and maximum longitude values
+//! * ymin, ymax: the minimum and maximum latitude values
+//! * zmin, zmax: (optional) the minimum and maximum elevation values
+//! * mmin, mmax: (optional) the minimum and maximum linear reference values
+//!
+//! In 2D representation, where x are points:
+//!       
+//!  ymax +-----------------------+
+//!       |               x       |
+//!       |      x                |
+//!       |              x        |
+//!       |      x                |
+//!  ymin +-----------------------+
+//!       xmin                    xmax

Review Comment:
   It might render better like this:
   
   ```suggestion
   //! ```text
   //!  ymax +-----------------------+
   //!       |               x       |
   //!       |      x                |
   //!       |              x        |
   //!       |      x                |
   //!  ymin +-----------------------+
   //!       xmin                    xmax
   //! ```
   ```



##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1969,9 +1986,9 @@ mod tests {
             .build();
 
         #[cfg(not(feature = "encryption"))]
-        let base_expected_size = 2312;
+        let base_expected_size = 2792;

Review Comment:
   this is a fairly non trivial increase in size (as the parquet metadata size is already a challenge for many users)
   
   Can we please reduce this size by `Box`ing the Geo statistics? Something like
   
   instead of 
   ```rust
       geo_statistics: Option<geo_statistics::GeospatialStatistics>,
   ```
   
   Make it like
   ```rust
       geo_statistics: Option<Box<geo_statistics::GeospatialStatistics>>,
   ```
   
   (that would add just 8-16 bytes for another pointer rather than the size of the inlined structure)



##########
parquet/src/geospatial/bounding_box.rs:
##########
@@ -0,0 +1,288 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bounding box for GEOMETRY or GEOGRAPHY type in the representation of min/max
+//! value pair of coordinates from each axis.
+//!
+//! Derived from the parquet format spec: <https://github.com/apache/parquet-format/blob/master/Geospatial.md>
+//!
+//!
+use crate::format as parquet;
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound can occur when the bounding box crosses the antimeridian line.
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+///
+/// Derived from the parquet format [spec][bounding-box-spec]
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+///
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0);
+///
+/// // 3D bounding box with elevation and measured value
+/// let bbox_3d_m = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0)
+///     .with_mrange(0.0, 1000.0);
+/// ```
+///
+/// [bounding-box-spec]: https://github.com/apache/parquet-format/blob/master/Geospatial.md#bounding-box
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    pub fn new(xmin: f64, xmax: f64, ymin: f64, ymax: f64) -> Self {
+        Self {
+            xmin,
+            xmax,
+            ymin,
+            ymax,
+            zmin: None,
+            zmax: None,
+            mmin: None,
+            mmax: None,
+        }
+    }
+
+    /// Updates the bounding box with specified X-coordinate range.
+    pub fn with_xrange(mut self, xmin: f64, xmax: f64) -> Self {
+        self.xmin = xmin;
+        self.xmax = xmax;
+        self
+    }
+
+    /// Updates the bounding box with specified Y-coordinate range.
+    pub fn with_yrange(mut self, ymin: f64, ymax: f64) -> Self {
+        self.ymin = ymin;
+        self.ymax = ymax;
+        self
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    pub fn with_zrange(mut self, zmin: f64, zmax: f64) -> Self {
+        self.zmin = Some(zmin);
+        self.zmax = Some(zmax);
+        self
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    pub fn with_mrange(mut self, mmin: f64, mmax: f64) -> Self {
+        self.mmin = Some(mmin);
+        self.mmax = Some(mmax);
+        self
+    }
+
+    /// Returns the minimum x-coordinate.
+    pub fn get_xmin(&self) -> f64 {
+        self.xmin
+    }
+
+    /// Returns the maximum x-coordinate.
+    pub fn get_xmax(&self) -> f64 {
+        self.xmax
+    }
+
+    /// Returns the minimum y-coordinate.
+    pub fn get_ymin(&self) -> f64 {
+        self.ymin
+    }
+
+    /// Returns the maximum y-coordinate.
+    pub fn get_ymax(&self) -> f64 {
+        self.ymax
+    }
+
+    /// Returns the minimum z-coordinate, if present.
+    pub fn get_zmin(&self) -> Option<f64> {
+        self.zmin
+    }
+
+    /// Returns the maximum z-coordinate, if present.
+    pub fn get_zmax(&self) -> Option<f64> {
+        self.zmax
+    }
+
+    /// Returns the minimum m-value (measure), if present.
+    pub fn get_mmin(&self) -> Option<f64> {
+        self.mmin
+    }
+
+    /// Returns the maximum m-value (measure), if present.
+    pub fn get_mmax(&self) -> Option<f64> {
+        self.mmax
+    }
+
+    /// Returns `true` if both zmin and zmax are present.
+    pub fn is_z_valid(&self) -> bool {
+        self.zmin.is_some() && self.zmax.is_some()
+    }
+
+    /// Returns `true` if both mmin and mmax are present.
+    pub fn is_m_valid(&self) -> bool {
+        self.mmin.is_some() && self.mmax.is_some()
+    }
+}
+
+impl From<BoundingBox> for parquet::BoundingBox {
+    /// Converts our internal `BoundingBox` to the Thrift-generated format.
+    fn from(b: BoundingBox) -> parquet::BoundingBox {
+        parquet::BoundingBox {
+            xmin: b.get_xmin().into(),
+            ymin: b.get_ymin().into(),
+            xmax: b.get_xmax().into(),
+            ymax: b.get_ymax().into(),
+            zmin: b.get_zmin().map(|z| z.into()),
+            zmax: b.get_zmax().map(|z| z.into()),
+            mmin: b.get_mmin().map(|m| m.into()),
+            mmax: b.get_mmax().map(|m| m.into()),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_bounding_box() {
+        let bbox = BoundingBox::new(0.0, 0.0, 10.0, 10.0);
+        assert_eq!(bbox.get_xmin(), 0.0);
+        assert_eq!(bbox.get_xmax(), 0.0);
+        assert_eq!(bbox.get_ymin(), 10.0);
+        assert_eq!(bbox.get_ymax(), 10.0);
+        assert_eq!(bbox.get_zmin(), None);
+        assert_eq!(bbox.get_zmax(), None);
+        assert_eq!(bbox.get_mmin(), None);
+        assert_eq!(bbox.get_mmax(), None);
+        assert!(!bbox.is_z_valid());
+        assert!(!bbox.is_m_valid());
+
+        // test with zrange
+        let bbox_z = BoundingBox::new(0.0, 0.0, 10.0, 10.0).with_zrange(5.0, 15.0);
+        assert_eq!(bbox_z.get_zmin(), Some(5.0));
+        assert_eq!(bbox_z.get_zmax(), Some(15.0));
+        assert!(bbox_z.is_z_valid());
+        assert!(!bbox_z.is_m_valid());
+
+        // test with mrange
+        let bbox_m = BoundingBox::new(0.0, 0.0, 10.0, 10.0).with_mrange(10.0, 20.0);
+        assert_eq!(bbox_m.get_mmin(), Some(10.0));
+        assert_eq!(bbox_m.get_mmax(), Some(20.0));
+        assert!(!bbox_m.is_z_valid());
+        assert!(bbox_m.is_m_valid());
+
+        // test with zrange and mrange
+        let bbox_zm = BoundingBox::new(0.0, 0.0, 10.0, 10.0)
+            .with_zrange(5.0, 15.0)
+            .with_mrange(10.0, 20.0);
+        assert_eq!(bbox_zm.get_zmin(), Some(5.0));
+        assert_eq!(bbox_zm.get_zmax(), Some(15.0));
+        assert_eq!(bbox_zm.get_mmin(), Some(10.0));
+        assert_eq!(bbox_zm.get_mmax(), Some(20.0));
+        assert!(bbox_zm.is_z_valid());
+        assert!(bbox_zm.is_m_valid());
+    }
+
+    #[test]
+    fn test_bounding_box_to_thrift() {

Review Comment:
   Another good set of tests for thrift encoding/decoding would be "roundtrip" tests that create a thrift structure -> rust --> thrift and ensure all fields survive the round trip



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3343354449

   I think we have bikeshed this one enough -- than you @kaushiksrini and @kylebarron  @BlakeOrth and @paleolimbot 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "BlakeOrth (via GitHub)" <gi...@apache.org>.
BlakeOrth commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2301714643


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,320 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::{ParquetError, Result};
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound occurs when the bounding box crosses the antimeridian line
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0).with_zrange(0.0, 1000.0);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64) -> Self {
+        Self { xmin, ymin, xmax, ymax, zmin: None, zmax: None, mmin: None, mmax: None }
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `zmin` - Minimum Z coordinate
+    /// * `zmax` - Maximum Z coordinate
+    pub fn with_zrange(self, zmin: f64, zmax: f64) -> Self {
+        Self { zmin: Some(zmin), zmax: Some(zmax), ..self }
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `mmin` - Minimum M coordinate
+    /// * `mmax` - Maximum M coordinate
+    pub fn with_mrange(self, mmin: f64, mmax: f64) -> Self {
+        Self { mmin: Some(mmin), mmax: Some(mmax), ..self }
+    }
+}
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+/// 
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::{GeospatialStatistics, BoundingBox};
+/// 
+/// // Empty statistics
+/// let empty_stats = GeospatialStatistics::new_empty();
+/// 
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding box encompassing all geospatial data
+    bbox: Option<BoundingBox>,
+    /// Optional list of geospatial geometry type identifiers
+    /// as specified in https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#geospatial-types
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   Should these be `u32` instead of `i32`?
   
   https://en.wikipedia.org/wiki/Well-known_text_representation_of_geometry#Well-known_binary



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2301815392


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,320 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::{ParquetError, Result};
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound occurs when the bounding box crosses the antimeridian line
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0).with_zrange(0.0, 1000.0);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64) -> Self {
+        Self { xmin, ymin, xmax, ymax, zmin: None, zmax: None, mmin: None, mmax: None }
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `zmin` - Minimum Z coordinate
+    /// * `zmax` - Maximum Z coordinate
+    pub fn with_zrange(self, zmin: f64, zmax: f64) -> Self {
+        Self { zmin: Some(zmin), zmax: Some(zmax), ..self }
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `mmin` - Minimum M coordinate
+    /// * `mmax` - Maximum M coordinate
+    pub fn with_mrange(self, mmin: f64, mmax: f64) -> Self {
+        Self { mmin: Some(mmin), mmax: Some(mmax), ..self }
+    }
+}
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+/// 
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::{GeospatialStatistics, BoundingBox};
+/// 
+/// // Empty statistics
+/// let empty_stats = GeospatialStatistics::new_empty();
+/// 
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding box encompassing all geospatial data
+    bbox: Option<BoundingBox>,
+    /// Optional list of geospatial geometry type identifiers
+    /// as specified in https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#geospatial-types
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   You could make it `u16` if you wanted.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3303671863

   I agree that makes sense. Perhaps we should start with a PR just setting up that crate?


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2373747046


##########
parquet/src/geospatial/mod.rs:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! This module provides functionality for working with geospatial data in Parquet file as defied in the [spec][parquet-geo-spec].

Review Comment:
   ```suggestion
   //! This module provides functionality for working with geospatial data in Parquet file as defined in the [spec][parquet-geo-spec].
   ```



##########
parquet/src/geospatial/bounding_box.rs:
##########
@@ -0,0 +1,429 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bounding box for GEOMETRY or GEOGRAPHY type in the representation of min/max
+//! value pair of coordinates from each axis.
+//!
+//! Derived from the parquet format spec: <https://github.com/apache/parquet-format/blob/master/Geospatial.md>
+//!
+//!
+use crate::format as parquet;
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound can occur when the bounding box crosses the antimeridian line.
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+///
+/// Derived from the parquet format [spec][bounding-box-spec]
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+///
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0);
+///
+/// // 3D bounding box with elevation and measured value
+/// let bbox_3d_m = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0)
+///     .with_mrange(0.0, 1000.0);
+/// ```
+///
+/// [bounding-box-spec]: https://github.com/apache/parquet-format/blob/master/Geospatial.md#bounding-box
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    pub fn new(xmin: f64, xmax: f64, ymin: f64, ymax: f64) -> Self {
+        Self {
+            xmin,
+            xmax,
+            ymin,
+            ymax,
+            zmin: None,
+            zmax: None,
+            mmin: None,
+            mmax: None,
+        }
+    }
+
+    /// Updates the bounding box with specified X-coordinate range.
+    pub fn with_xrange(mut self, xmin: f64, xmax: f64) -> Self {
+        self.xmin = xmin;
+        self.xmax = xmax;
+        self
+    }
+
+    /// Updates the bounding box with specified Y-coordinate range.
+    pub fn with_yrange(mut self, ymin: f64, ymax: f64) -> Self {
+        self.ymin = ymin;
+        self.ymax = ymax;
+        self
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    pub fn with_zrange(mut self, zmin: f64, zmax: f64) -> Self {
+        self.zmin = Some(zmin);
+        self.zmax = Some(zmax);
+        self
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    pub fn with_mrange(mut self, mmin: f64, mmax: f64) -> Self {
+        self.mmin = Some(mmin);
+        self.mmax = Some(mmax);
+        self
+    }
+
+    /// Returns the minimum x-coordinate.
+    pub fn get_xmin(&self) -> f64 {
+        self.xmin
+    }
+
+    /// Returns the maximum x-coordinate.
+    pub fn get_xmax(&self) -> f64 {
+        self.xmax
+    }
+
+    /// Returns the minimum y-coordinate.
+    pub fn get_ymin(&self) -> f64 {
+        self.ymin
+    }
+
+    /// Returns the maximum y-coordinate.
+    pub fn get_ymax(&self) -> f64 {
+        self.ymax
+    }
+
+    /// Returns the minimum z-coordinate, if present.
+    pub fn get_zmin(&self) -> Option<f64> {
+        self.zmin
+    }
+
+    /// Returns the maximum z-coordinate, if present.
+    pub fn get_zmax(&self) -> Option<f64> {
+        self.zmax
+    }
+
+    /// Returns the minimum m-value (measure), if present.
+    pub fn get_mmin(&self) -> Option<f64> {
+        self.mmin
+    }
+
+    /// Returns the maximum m-value (measure), if present.
+    pub fn get_mmax(&self) -> Option<f64> {
+        self.mmax
+    }
+
+    /// Returns `true` if both zmin and zmax are present.
+    pub fn is_z_valid(&self) -> bool {
+        self.zmin.is_some() && self.zmax.is_some()
+    }
+
+    /// Returns `true` if both mmin and mmax are present.
+    pub fn is_m_valid(&self) -> bool {
+        self.mmin.is_some() && self.mmax.is_some()
+    }
+}
+
+impl From<BoundingBox> for parquet::BoundingBox {
+    /// Converts our internal `BoundingBox` to the Thrift-generated format.
+    fn from(b: BoundingBox) -> parquet::BoundingBox {
+        parquet::BoundingBox {
+            xmin: b.get_xmin().into(),

Review Comment:
   Tiny nit: since we have access to the private attributes here, it might be slightly more readable to use those, but up to you:
   
   ```suggestion
               xmin: b.xmin.into(),
   ```



##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1430,6 +1444,12 @@ impl ColumnChunkMetaDataBuilder {
         self
     }
 
+    /// Sets geospatial statistics for this column chunk.
+    pub fn set_geo_statistics(mut self, value: geo_statistics::GeospatialStatistics) -> Self {
+        self.0.geo_statistics = Some(Box::new(value));

Review Comment:
   Do we want the `Box` only to be an implementation detail? If the user already has a `GeospatialStatistics`, then this has to be copied in `Box::new`? 
   
   I'm just wondering whether there's any value in having the API take in a `Box<GeospatialStatistics>` instead of boxing it inside this function



##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,307 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::{ParquetError, Result};
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound occurs when the bounding box crosses the antimeridian line
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0).with_zrange(0.0, 1000.0);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present

Review Comment:
   I like the idea of using tuples here



##########
parquet/src/geospatial/bounding_box.rs:
##########
@@ -0,0 +1,429 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bounding box for GEOMETRY or GEOGRAPHY type in the representation of min/max
+//! value pair of coordinates from each axis.
+//!
+//! Derived from the parquet format spec: <https://github.com/apache/parquet-format/blob/master/Geospatial.md>
+//!
+//!
+use crate::format as parquet;
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound can occur when the bounding box crosses the antimeridian line.
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+///
+/// Derived from the parquet format [spec][bounding-box-spec]
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+///
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0);
+///
+/// // 3D bounding box with elevation and measured value
+/// let bbox_3d_m = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0)
+///     .with_mrange(0.0, 1000.0);
+/// ```
+///
+/// [bounding-box-spec]: https://github.com/apache/parquet-format/blob/master/Geospatial.md#bounding-box
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    pub fn new(xmin: f64, xmax: f64, ymin: f64, ymax: f64) -> Self {
+        Self {
+            xmin,
+            xmax,
+            ymin,
+            ymax,
+            zmin: None,
+            zmax: None,
+            mmin: None,
+            mmax: None,
+        }
+    }
+
+    /// Updates the bounding box with specified X-coordinate range.
+    pub fn with_xrange(mut self, xmin: f64, xmax: f64) -> Self {
+        self.xmin = xmin;
+        self.xmax = xmax;
+        self
+    }
+
+    /// Updates the bounding box with specified Y-coordinate range.
+    pub fn with_yrange(mut self, ymin: f64, ymax: f64) -> Self {
+        self.ymin = ymin;
+        self.ymax = ymax;
+        self
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    pub fn with_zrange(mut self, zmin: f64, zmax: f64) -> Self {
+        self.zmin = Some(zmin);
+        self.zmax = Some(zmax);
+        self
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    pub fn with_mrange(mut self, mmin: f64, mmax: f64) -> Self {
+        self.mmin = Some(mmin);
+        self.mmax = Some(mmax);
+        self
+    }
+
+    /// Returns the minimum x-coordinate.
+    pub fn get_xmin(&self) -> f64 {
+        self.xmin
+    }
+
+    /// Returns the maximum x-coordinate.
+    pub fn get_xmax(&self) -> f64 {
+        self.xmax
+    }
+
+    /// Returns the minimum y-coordinate.
+    pub fn get_ymin(&self) -> f64 {
+        self.ymin
+    }
+
+    /// Returns the maximum y-coordinate.
+    pub fn get_ymax(&self) -> f64 {
+        self.ymax
+    }
+
+    /// Returns the minimum z-coordinate, if present.
+    pub fn get_zmin(&self) -> Option<f64> {
+        self.zmin
+    }
+
+    /// Returns the maximum z-coordinate, if present.
+    pub fn get_zmax(&self) -> Option<f64> {
+        self.zmax
+    }
+
+    /// Returns the minimum m-value (measure), if present.
+    pub fn get_mmin(&self) -> Option<f64> {
+        self.mmin
+    }
+
+    /// Returns the maximum m-value (measure), if present.
+    pub fn get_mmax(&self) -> Option<f64> {
+        self.mmax
+    }
+
+    /// Returns `true` if both zmin and zmax are present.
+    pub fn is_z_valid(&self) -> bool {
+        self.zmin.is_some() && self.zmax.is_some()
+    }
+
+    /// Returns `true` if both mmin and mmax are present.
+    pub fn is_m_valid(&self) -> bool {
+        self.mmin.is_some() && self.mmax.is_some()
+    }
+}
+
+impl From<BoundingBox> for parquet::BoundingBox {
+    /// Converts our internal `BoundingBox` to the Thrift-generated format.
+    fn from(b: BoundingBox) -> parquet::BoundingBox {
+        parquet::BoundingBox {
+            xmin: b.get_xmin().into(),
+            ymin: b.get_ymin().into(),
+            xmax: b.get_xmax().into(),
+            ymax: b.get_ymax().into(),
+            zmin: b.get_zmin().map(|z| z.into()),
+            zmax: b.get_zmax().map(|z| z.into()),
+            mmin: b.get_mmin().map(|m| m.into()),
+            mmax: b.get_mmax().map(|m| m.into()),
+        }
+    }
+}
+
+impl From<parquet::BoundingBox> for BoundingBox {
+    fn from(bbox: parquet::BoundingBox) -> Self {
+        let mut new_bbox = Self::new(
+            bbox.xmin.into(),
+            bbox.xmax.into(),
+            bbox.ymin.into(),
+            bbox.ymax.into(),
+        );
+
+        new_bbox = match (bbox.zmin, bbox.zmax) {
+            (Some(zmin), Some(zmax)) => new_bbox.with_zrange(zmin.into(), zmax.into()),
+            // If both None or mismatch, set it to None and don't error

Review Comment:
   Here and below, slight wording suggestion
   ```suggestion
               // If either None or mismatch, leave it as None and don't error
   ```



##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::errors::Result;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::geospatial::bounding_box::BoundingBox;
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+///
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::statistics::GeospatialStatistics;
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding defining the spatial extent, where None represents a lack of information.
+    bbox: Option<BoundingBox>,
+    /// Optional list of geometry type identifiers, where None represents lack of information
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   I think in the future we may want a more strongly-typed enum than a bare `i32`, but this is fine for now.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3332036261

   Thanks @kylebarron for the review! Implemented the suggestions


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3303706745

   > Does this allow us to potentially make breaking changes outside of the normal release cycle for this crate since it's behind an opt-in "experimental" feature flag? I suspect that could help speed along the initial development effort if so (I can say that I rarely get an API "right" the first time through and it often either wants or needs breaking changes as it matures).
   
   Yes, exactly, we are going to call the feature flag experimental until we are reasonably confident the API looks good and we can commit to stabalizing it. 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2374177575


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::errors::Result;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::geospatial::bounding_box::BoundingBox;
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+///
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::statistics::GeospatialStatistics;
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding defining the spatial extent, where None represents a lack of information.
+    bbox: Option<BoundingBox>,
+    /// Optional list of geometry type identifiers, where None represents lack of information
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   More strongly-typed enums are possibly the work of the geoarrow-rs and Sedonas of the world (where interested clients can convert the identifiers into nice structures if they need to). I think here we're just trying to provide the bare minimum for other libraries to be able to access the statistics.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2373735885


##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1295,7 +1306,9 @@ impl ColumnChunkMetaData {
             bloom_filter_offset: self.bloom_filter_offset,
             bloom_filter_length: self.bloom_filter_length,
             size_statistics,
-            geospatial_statistics: None,
+            geospatial_statistics: geo_statistics::to_thrift(
+                self.geo_statistics.as_ref().map(|boxed| boxed.as_ref()),

Review Comment:
   ```suggestion
                   self.geo_statistics.as_deref(),
   ```



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2356347469


##########
parquet/src/geospatial/bounding_box.rs:
##########
@@ -0,0 +1,359 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bounding box for GEOMETRY or GEOGRAPHY type in the representation of min/max
+//! value pair of coordinates from each axis.
+//! 
+//! Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/master/Geospatial.md
+//! 
+//! 
+use crate::format as parquet;
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound can occur when the bounding box crosses the antimeridian line.
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format [spec][bounding-box-spec]
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::bounding_box::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0);
+/// 
+/// // 3D bounding box with elevation and measured value
+/// let bbox_3d_m = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0)
+///     .with_mrange(0.0, 1000.0);
+/// ```
+/// 
+/// [bounding-box-spec]: https://github.com/apache/parquet-format/blob/master/Geospatial.md#bounding-box
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {

Review Comment:
   There are some subtle correctness issues that can arise with the BoundingBox with respect to the known or unknown-ness of the Z or M dimensions and the "x_wraparound" situation.
   
   I would suggest removing the `update_xxx()` and `merge_xxx()` methods for now, because they are not strictly correct in the presence of a wraparound X interval. In other words, I would scope this BoundingBox to be strictly a way for a user to access the contents of a Column Chunk's statistics.
   
   The building of bounding boxes, particularly in the presence of geography and/or wraparound, is rather complicated. You are welcome to lift any of the code I wrote in SedonaDB for this when the time comes (but again, I'd suggest that time is later): https://github.com/apache/sedona-db/blob/653ab44bdd2923b5c395828f93de7fc3085ff6c2/rust/sedona-geometry/src/interval.rs



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2383720272


##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1430,6 +1444,12 @@ impl ColumnChunkMetaDataBuilder {
         self
     }
 
+    /// Sets geospatial statistics for this column chunk.
+    pub fn set_geo_statistics(mut self, value: geo_statistics::GeospatialStatistics) -> Self {
+        self.0.geo_statistics = Some(Box::new(value));

Review Comment:
   Got it! I changed it to a take in a heap allocated object



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2299561358


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,290 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::Result;
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// Represents a 2D/3D bounding box with optional M-coordinate support.
+/// 
+/// A bounding box defines the spatial extent of geospatial data by specifying
+/// minimum and maximum coordinates along each axis. This struct supports:
+/// - 2D coordinates (x, y) which are always required
+/// - Optional 3D coordinates (z) for elevation/height data
+/// - Optional M-coordinates for measured values (e.g., distance along a route)
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, Some(0.0), Some(1000.0), None, None);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// * `zmin` - Optional minimum Z coordinate
+    /// * `zmax` - Optional maximum Z coordinate
+    /// * `mmin` - Optional minimum M coordinate
+    /// * `mmax` - Optional maximum M coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64, zmin: Option<f64>, zmax: Option<f64>, mmin: Option<f64>, mmax: Option<f64>) -> Self {
+        Self { xmin, ymin, xmax, ymax, zmin, zmax, mmin, mmax }
+    }
+}
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+/// 
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::{GeospatialStatistics, BoundingBox};
+/// 
+/// // Empty statistics
+/// let empty_stats = GeospatialStatistics::new_empty();
+/// 
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct GeospatialStatistics {
+    /// Optional bounding box encompassing all geospatial data
+    bbox: Option<BoundingBox>,
+    /// Optional list of geospatial geometry type identifiers
+    /// 
+    /// Common values include:
+    /// - 1: Point
+    /// - 2: LineString  
+    /// - 3: Polygon
+    /// - 4: MultiPoint
+    /// - 5: MultiLineString
+    /// - 6: MultiPolygon
+    /// - 7: GeometryCollection
+    geospatial_types: Option<Vec<i32>>,
+}
+
+impl GeospatialStatistics {
+    /// Creates a new empty geospatial statistics instance.
+    /// 
+    /// This is useful when no spatial information is available or when
+    /// creating a placeholder for statistics that will be populated later.
+    /// 
+    /// # Returns
+    /// 
+    /// A `GeospatialStatistics` instance with no bounding box or type information.
+    pub fn new_empty() -> Self {

Review Comment:
   Maybe cleaner to remove this and just implement `Default`?



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2299559348


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,290 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::Result;
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// Represents a 2D/3D bounding box with optional M-coordinate support.
+/// 
+/// A bounding box defines the spatial extent of geospatial data by specifying
+/// minimum and maximum coordinates along each axis. This struct supports:
+/// - 2D coordinates (x, y) which are always required
+/// - Optional 3D coordinates (z) for elevation/height data
+/// - Optional M-coordinates for measured values (e.g., distance along a route)
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, Some(0.0), Some(1000.0), None, None);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// * `zmin` - Optional minimum Z coordinate
+    /// * `zmax` - Optional maximum Z coordinate
+    /// * `mmin` - Optional minimum M coordinate
+    /// * `mmax` - Optional maximum M coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64, zmin: Option<f64>, zmax: Option<f64>, mmin: Option<f64>, mmax: Option<f64>) -> Self {
+        Self { xmin, ymin, xmax, ymax, zmin, zmax, mmin, mmax }
+    }
+}
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+/// 
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::{GeospatialStatistics, BoundingBox};
+/// 
+/// // Empty statistics
+/// let empty_stats = GeospatialStatistics::new_empty();
+/// 
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct GeospatialStatistics {
+    /// Optional bounding box encompassing all geospatial data
+    bbox: Option<BoundingBox>,
+    /// Optional list of geospatial geometry type identifiers
+    /// 
+    /// Common values include:
+    /// - 1: Point
+    /// - 2: LineString  
+    /// - 3: Polygon
+    /// - 4: MultiPoint
+    /// - 5: MultiLineString
+    /// - 6: MultiPolygon
+    /// - 7: GeometryCollection

Review Comment:
   I'd suggest also linking to https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#geospatial-types for the full allowed list of type ids.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3222390131

   Thanks @kylebarron for the feedback - made some changes


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2380137202


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::errors::Result;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::geospatial::bounding_box::BoundingBox;
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+///
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::statistics::GeospatialStatistics;
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding defining the spatial extent, where None represents a lack of information.
+    bbox: Option<BoundingBox>,
+    /// Optional list of geometry type identifiers, where None represents lack of information
+    geospatial_types: Option<Vec<i32>>,
+}
+
+impl GeospatialStatistics {
+    /// Creates a new geospatial statistics instance with the specified data.
+    pub fn new(bbox: Option<BoundingBox>, geospatial_types: Option<Vec<i32>>) -> Self {
+        Self {
+            bbox,
+            geospatial_types,
+        }
+    }
+}
+
+/// Converts a Thrift-generated geospatial statistics object to the internal representation.
+pub fn from_thrift(
+    geo_statistics: Option<TGeospatialStatistics>,
+) -> Result<Option<GeospatialStatistics>> {

Review Comment:
   since this is infallable (always returns `Ok`) maybe we could make the `from_thrift` function just return `Option` 
   
   That would also let you simplify the body if you wanted
   ```rust
   pub fn from_thrift(
       geo_statistics: Option<TGeospatialStatistics>,
   ) -> Option<GeospatialStatistics> {
       let geo_stats = geo_statistics?;
       let bbox = geo_stats.bbox.map(|bbox| bbox.into());
       // If vector is empty, then set it to None
       let geospatial_types: Option<Vec<i32>> =
           geo_stats.geospatial_types.filter(|v| !v.is_empty());
       Some(GeospatialStatistics::new(bbox, geospatial_types))
   }
   ```
   
   



##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1430,6 +1444,12 @@ impl ColumnChunkMetaDataBuilder {
         self
     }
 
+    /// Sets geospatial statistics for this column chunk.
+    pub fn set_geo_statistics(mut self, value: geo_statistics::GeospatialStatistics) -> Self {
+        self.0.geo_statistics = Some(Box::new(value));

Review Comment:
   I think the point would be if a user already has a `Box<GeospatialStatistics>` they could pass it in which would be a pointer copy, rather than this which might have to move data from the stack to the heap
   
   



##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::errors::Result;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::geospatial::bounding_box::BoundingBox;
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+///
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+///
+/// # Examples
+///
+/// ```
+/// use parquet::geospatial::statistics::GeospatialStatistics;
+/// use parquet::geospatial::bounding_box::BoundingBox;
+///
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding defining the spatial extent, where None represents a lack of information.
+    bbox: Option<BoundingBox>,
+    /// Optional list of geometry type identifiers, where None represents lack of information
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   Since this is not a public field, we can also potentially change it in the future too, though I do see it is part of the constructor
   
   In my opinion it is fine to leave like this and we can adjust / update in a future release



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2299560346


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,290 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::Result;
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// Represents a 2D/3D bounding box with optional M-coordinate support.

Review Comment:
   Maybe best to just copy the full upstream docstring https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2299557494


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,290 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::Result;
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// Represents a 2D/3D bounding box with optional M-coordinate support.
+/// 
+/// A bounding box defines the spatial extent of geospatial data by specifying
+/// minimum and maximum coordinates along each axis. This struct supports:
+/// - 2D coordinates (x, y) which are always required
+/// - Optional 3D coordinates (z) for elevation/height data
+/// - Optional M-coordinates for measured values (e.g., distance along a route)
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0, Some(0.0), Some(1000.0), None, None);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// * `zmin` - Optional minimum Z coordinate
+    /// * `zmax` - Optional maximum Z coordinate
+    /// * `mmin` - Optional minimum M coordinate
+    /// * `mmax` - Optional maximum M coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64, zmin: Option<f64>, zmax: Option<f64>, mmin: Option<f64>, mmax: Option<f64>) -> Self {

Review Comment:
   Instead of having this very verbose constructor with 8 arguments, I'd suggest having `new()` take only the required 4 values for the 2D box. Then have a `with_zrange` that takes in non-null zmin and zmax and have a `with_mrange` that takes in non-null mmin and mmax. This also ensures that it's impossible to define a null zmin with non-null zmax.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2357426144


##########
parquet/src/geospatial/bounding_box.rs:
##########
@@ -0,0 +1,359 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bounding box for GEOMETRY or GEOGRAPHY type in the representation of min/max
+//! value pair of coordinates from each axis.
+//! 
+//! Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/master/Geospatial.md
+//! 
+//! 
+use crate::format as parquet;
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound can occur when the bounding box crosses the antimeridian line.
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format [spec][bounding-box-spec]
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::bounding_box::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0);
+/// 
+/// // 3D bounding box with elevation and measured value
+/// let bbox_3d_m = BoundingBox::new(0.0, 0.0, 100.0, 100.0)
+///     .with_zrange(0.0, 1000.0)
+///     .with_mrange(0.0, 1000.0);
+/// ```
+/// 
+/// [bounding-box-spec]: https://github.com/apache/parquet-format/blob/master/Geospatial.md#bounding-box
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {

Review Comment:
   Thanks @paleolimbot for the catch! That makes sense - I have removed it here



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3316676418

   Thank you for the feedback @alamb and @paleolimbot! I’ve made the suggested changes — please let me know if there’s anything else I should address.


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2373734616


##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1061,6 +1063,12 @@ impl ColumnChunkMetaData {
         self.statistics.as_ref()
     }
 
+    /// Returns geospatial statistics that are set for this column chunk,
+    /// or `None` if no geospatial statistics are available.
+    pub fn geo_statistics(&self) -> Option<&geo_statistics::GeospatialStatistics> {
+        self.geo_statistics.as_ref().map(|boxed| boxed.as_ref())

Review Comment:
   ```suggestion
           self.geo_statistics.as_deref()
   ```



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2374154878


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,307 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::{ParquetError, Result};
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound occurs when the bounding box crosses the antimeridian line
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0).with_zrange(0.0, 1000.0);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present

Review Comment:
   I'm OK with either here (they are not `pub` and the accessors all need to be there anyway). This really is how the object is defined for Thrift.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2377654704


##########
parquet/src/file/metadata/mod.rs:
##########
@@ -1430,6 +1444,12 @@ impl ColumnChunkMetaDataBuilder {
         self
     }
 
+    /// Sets geospatial statistics for this column chunk.
+    pub fn set_geo_statistics(mut self, value: geo_statistics::GeospatialStatistics) -> Self {
+        self.0.geo_statistics = Some(Box::new(value));

Review Comment:
   Left it as is since I felt the user doesn't necessarily need to know the memory-level implementation details, and there's no performance difference (`Box::new` moves rather than copies). However, open to suggestions!



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kaushiksrini (via GitHub)" <gi...@apache.org>.
kaushiksrini commented on code in PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#discussion_r2302304044


##########
parquet/src/geospatial/statistics.rs:
##########
@@ -0,0 +1,320 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial statistics for Parquet files.
+//!
+//! This module provides functionality for working with geospatial statistics in Parquet files.
+//! It includes support for bounding boxes and geospatial statistics in column chunk metadata.
+
+use crate::format as parquet;
+use crate::format::GeospatialStatistics as TGeospatialStatistics;
+use crate::errors::{ParquetError, Result};
+
+// ----------------------------------------------------------------------
+// Bounding Box
+
+/// A geospatial instance has at least two coordinate dimensions: X and Y for 2D coordinates of each point.
+/// X represents longitude/easting and Y represents latitude/northing. A geospatial instance can optionally
+/// have Z and/or M values associated with each point.
+///
+/// The Z values introduce the third dimension coordinate, typically used to indicate height or elevation.
+///
+/// M values allow tracking a value in a fourth dimension. These can represent:
+/// - Linear reference values (e.g., highway milepost)
+/// - Timestamps
+/// - Other values defined by the CRS
+///
+/// The bounding box is defined as min/max value pairs of coordinates from each axis. X and Y values are
+/// always present, while Z and M are omitted for 2D geospatial instances.
+///
+/// When calculating a bounding box:
+/// - Null or NaN values in a coordinate dimension are skipped
+/// - If a dimension has only null/NaN values, that dimension is omitted
+/// - If either X or Y dimension is missing, no bounding box is produced
+/// - Example: POINT (1 NaN) contributes to X but not to Y, Z, or M dimensions
+///
+/// Special cases:
+/// - For X values only, xmin may exceed xmax. In this case, a point matches if x >= xmin OR x <= xmax
+/// - This wraparound occurs when the bounding box crosses the antimeridian line
+/// - In geographic terms: xmin=westernmost, xmax=easternmost, ymin=southernmost, ymax=northernmost
+///
+/// For GEOGRAPHY types:
+/// - X values must be within [-180, 180] (longitude)
+/// - Y values must be within [-90, 90] (latitude)
+/// 
+/// Derived from the parquet format spec: https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#bounding-box
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::BoundingBox;
+/// 
+/// // 2D bounding box
+/// let bbox_2d = BoundingBox::new(0.0, 0.0, 100.0, 100.0);
+/// 
+/// // 3D bounding box with elevation
+/// let bbox_3d = BoundingBox::new(0.0, 0.0, 100.0, 100.0).with_zrange(0.0, 1000.0);
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct BoundingBox {
+    /// Minimum X coordinate (longitude or easting)
+    xmin: f64,
+    /// Minimum Y coordinate (latitude or northing)
+    ymin: f64,
+    /// Maximum X coordinate (longitude or easting)
+    xmax: f64,
+    /// Maximum Y coordinate (latitude or northing)
+    ymax: f64,
+    /// Minimum Z coordinate (elevation/height), if present
+    zmin: Option<f64>,
+    /// Maximum Z coordinate (elevation/height), if present
+    zmax: Option<f64>,
+    /// Minimum M coordinate (measured value), if present
+    mmin: Option<f64>,
+    /// Maximum M coordinate (measured value), if present
+    mmax: Option<f64>,
+}
+
+impl BoundingBox {
+    /// Creates a new bounding box with the specified coordinates.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `xmin` - Minimum X coordinate
+    /// * `ymin` - Minimum Y coordinate  
+    /// * `xmax` - Maximum X coordinate
+    /// * `ymax` - Maximum Y coordinate
+    /// 
+    /// # Returns
+    /// 
+    /// A new `BoundingBox` instance with the specified coordinates.
+    pub fn new(xmin: f64, ymin: f64, xmax: f64, ymax: f64) -> Self {
+        Self { xmin, ymin, xmax, ymax, zmin: None, zmax: None, mmin: None, mmax: None }
+    }
+
+    /// Creates a new bounding box with the specified Z-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `zmin` - Minimum Z coordinate
+    /// * `zmax` - Maximum Z coordinate
+    pub fn with_zrange(self, zmin: f64, zmax: f64) -> Self {
+        Self { zmin: Some(zmin), zmax: Some(zmax), ..self }
+    }
+
+    /// Creates a new bounding box with the specified M-coordinate range.
+    /// 
+    /// # Arguments
+    /// 
+    /// * `mmin` - Minimum M coordinate
+    /// * `mmax` - Maximum M coordinate
+    pub fn with_mrange(self, mmin: f64, mmax: f64) -> Self {
+        Self { mmin: Some(mmin), mmax: Some(mmax), ..self }
+    }
+}
+
+// ----------------------------------------------------------------------
+// Geospatial Statistics
+
+/// Represents geospatial statistics for a Parquet column or dataset.
+/// 
+/// This struct contains metadata about the spatial characteristics of geospatial data,
+/// including bounding box information and the types of geospatial geometries present.
+/// It's used to optimize spatial queries and provide spatial context for data analysis.
+/// 
+/// # Examples
+/// 
+/// ```
+/// use parquet::geospatial::statistics::{GeospatialStatistics, BoundingBox};
+/// 
+/// // Empty statistics
+/// let empty_stats = GeospatialStatistics::new_empty();
+/// 
+/// // Statistics with bounding box
+/// let bbox = BoundingBox::new(0.0, 0.0, 100.0, 100.0, None, None, None, None);
+/// let stats = GeospatialStatistics::new(Some(bbox), Some(vec![1, 2, 3]));
+/// ```
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct GeospatialStatistics {
+    /// Optional bounding box encompassing all geospatial data
+    bbox: Option<BoundingBox>,
+    /// Optional list of geospatial geometry type identifiers
+    /// as specified in https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/Geospatial.md#geospatial-types
+    geospatial_types: Option<Vec<i32>>,

Review Comment:
   kept it as `i32` since to keep it in sync with the [thrift definition](https://github.com/apache/parquet-format/blob/ae39061f28d7c508a97af58a3c0a567352c8ea41/src/main/thrift/parquet.thrift#L260) and [c++ implementation](https://github.com/paleolimbot/arrow/blob/e1b7061cb1df3d5fc861fe80557cd7824980346a/cpp/src/parquet/geospatial/statistics.h#L58), but u16 makes more sense given the possible values



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [draft] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3222330359

   @alamb may have opinions on whether we should create a new feature flag for 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "BlakeOrth (via GitHub)" <gi...@apache.org>.
BlakeOrth commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3303701811

   > There is a new feature flag `variant_experimental`
   
   Does this allow us to potentially make breaking changes outside of the normal release cycle for this crate since it's behind an opt-in "experimental" feature flag? I suspect that could help speed along the initial development effort if so (I can say that I rarely get an API "right" the first time through and it often either wants or needs breaking changes as it matures).


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [geo] add geospatial statistics and bbox types for parquet [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8225:
URL: https://github.com/apache/arrow-rs/pull/8225#issuecomment-3304190280

   > Thank you all for taking a look!
   > 
   > > I think it would be really nice if users who didn't need to read geometry types could avoid paying the cost of that support (e.g. keep their binary and code size down).
   > 
   > I think that this is a great idea for the Arrow reader and writer (e.g., conversion to/from GeoArrow and writing statistics); however, can we at least provide access to the type annotation and statistics here? It seemed like that's where this PR was headed and I don't think the overhead of that is particularly onerous (I know I'm new here though!).
   > 
   > As a concrete target, I want to use this PR to prune row groups here:
   > 
   > https://github.com/apache/sedona-db/blob/653ab44bdd2923b5c395828f93de7fc3085ff6c2/rust/sedona-geoparquet/src/file_opener.rs#L186-L195
   > 
   > This is a place where I already have access to all the things I need (e.g., ParquetMetadata, file key/value metadata) and I don't really want or need that to be done for me. All I need is for `row_group_metadata.column(j).statistics()` to let me look at GeoStatistics.
   
   For sure -- if there are types that make sense to add (and always compile) to the main parquet crate sounds good to me. Reasonable rust structures for bounding box statistics sounds like it could fit this model
   
   What I am trying to avoid is having a bunch more code / binary size for users of the parquet crate if they aren't going to use geometry types, unless they opt in. 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org