Add Parquet geospatial statistics utility by paleolimbot · Pull Request #8414 · apache/arrow-rs

paleolimbot · 2025-09-22T22:21:12Z

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.

Closes Add geospatial statistics creation support for GEOMETRY/GEOGRAPHY Parquet logical types #8411.

Rationale for this change

The presence of relevant statistics when writing geometries and/or geographies is one of the primary motivations behind the GEOMETRY and/or GEOGRAPHY in Parquet. We'd like to make it easy for writers to provide them!

What changes are included in this PR?

This PR introduces Interval and WraparoundInterval structs that handle interval math, and a GeometryBounder that iterates over input using the fantastic wkb crate (via geo-traits).

Are these changes tested?

Yes!

Are there any user-facing changes?

All public structures and functions are documented (although I am not sure what the final public API will be).

kylebarron

It would be great to have more widespread docstrings, but otherwise this looks really good so far

kylebarron · 2025-09-24T00:38:12Z

parquet-geospatial/Cargo.toml

+arrow-schema = { workspace = true }
+geo-traits = { version = "0.3" }
+wkb = { version = "0.9" }
+
+[dev-dependencies]
+wkt = { version = "0.14" }


Do other crates have dependencies defined here? Or should we lift all dependencies up to the top-level Cargo.toml, even if they're only used by a single crate?

Other crates seem to 🤷

Happy to put these anywhere!

I think it is fine to put here, and if we end up with multiple versions in the workspace we can consolidate

alamb

Thank you @paleolimbot !

I had some small structural comments, but I don't really have enough experience with geometry to be able to detect logic errors or major mismatches in representation. I would (really) love to defer to @kylebarron for that level review

But from my perspective it would be fine to merge this PR and iterate as follow on PRs as well.

alamb · 2025-09-25T21:11:00Z

parquet-geospatial/Cargo.toml

+arrow-schema = { workspace = true }
+geo-traits = { version = "0.3" }
+wkb = { version = "0.9" }
+
+[dev-dependencies]
+wkt = { version = "0.14" }


I think it is fine to put here, and if we end up with multiple versions in the workspace we can consolidate

alamb · 2025-09-25T21:13:27Z

parquet-geospatial/src/bounding.rs

+
+use crate::interval::{Interval, IntervalTrait, WraparoundInterval};
+
+/// Geometry bounder


This comment implies to me that this method will be needed in the parquet writer/encoder itself, to accumulate the appropriate statistics.

I think that is fine, but it will take a bit of plumbing to sort out, as we won't have anything similar for Variant (Variant doesn't define any special statistics, so the writer will just treat it as a normal struct / binary array),

Yes, we will! (Perhaps we can wire that in at runtime as well)

alamb · 2025-09-25T21:14:42Z

parquet-geospatial/src/bounding.rs

+/// Parquet statistics with minimal modification.
+#[derive(Debug)]
+pub struct GeometryBounder {
+    x_left: Interval,


For someone familiar with geometry the meanings of x_left, x_mid and x_right are probably clear, but it might help someone who is not, like myself, if we could add a few more comments here

alamb · 2025-09-25T21:20:53Z

parquet-geospatial/src/bounding.rs

+    Ok(())
+}
+
+fn geometry_type(geom: &impl GeometryTrait<T = f64>) -> Result<i32, ArrowError> {


Can you add some reference about where these values came from? It seems maybe they are similar to the int32 values in #8225

It also seems strange to me that we are using i32 when we started with strongly typed enums (why not use enums again?)

Or maybe this should be a method on GeometryTrait 🤔

Probably the Wkb object could provide this. We could also compute it from the input bytes, but then we'd have to validate it which takes about as much code as this.

I don't really mind using enums but I don't think it helps us (#8225 can't depend on this because I think the idea is that this crate would be optional).

It would be nice to have an enum here. I get that it could make inter-crate workings more complex, so we can revisit it in the future

alamb · 2025-09-26T16:16:22Z

parquet-geospatial/src/bounding.rs

+/// (which adds some complexity to this implementation).
+#[derive(Debug)]
+pub struct GeometryBounder {
+    /// Union of all contiguous x intervals to the left of the wraparound midpoint


alamb · 2025-09-26T18:09:54Z

@kylebarron any further comments or concerns about merging this PR?

kylebarron

Thanks for all the docstrings! I haven't implemented this part of the spec before, so I also learned new things.

parquet-geospatial/src/bounding.rs

kylebarron · 2025-09-26T23:42:47Z

parquet-geospatial/src/bounding.rs

+        out
+    }
+
+    /// Update this bounder with one WKB-encoded geometry


Question for the future: will it ever make sense to be able to update the bounder before values have been encoded to WKB? It makes sense that it's simplest at this step of the process to intercept the WKB, though it's a small amount of overhead to have to parse the unaligned floats here, right? Maybe that amount of overhead isn't worth optimizing

I'm not sure! We can expose the geo-traits version as public if it comes up (or allow a caller to calculate the full GeoStatistics themselves).

kylebarron · 2025-09-26T23:45:02Z

parquet-geospatial/src/bounding.rs

+    Ok(())
+}
+
+fn geometry_type(geom: &impl GeometryTrait<T = f64>) -> Result<i32, ArrowError> {


It would be nice to have an enum here. I get that it could make inter-crate workings more complex, so we can revisit it in the future

Co-authored-by: Kyle Barron <kylebarron2@gmail.com>

alamb · 2025-09-28T12:58:02Z

Thanks @paleolimbot and @kylebarron 🚀

kylebarron reviewed Sep 24, 2025

View reviewed changes

paleolimbot changed the title ~~[WIP] Draft Parquet geospatial statistics creation~~ Add Parquet geospatial statistics utility Sep 24, 2025

paleolimbot marked this pull request as ready for review September 24, 2025 04:11

alamb approved these changes Sep 25, 2025

View reviewed changes

paleolimbot added 15 commits September 26, 2025 09:36

add interval

7c53ebf

basic bounding

01af5df

fmt

9f42efa

document the madness

50231be

geometry types

c2036f7

rename the visitor

7919407

fix recursion problem

66ba2b4

more docstrings

1a0a89b

more tests

2bf89d7

more docs

c6ada78

wkb

1a61d95

one missing test, one missing doc

e113079

docs

3f98501

doc link

f57001b

format?

a03090b

paleolimbot force-pushed the parquet-geospatial-bounding branch from a2a4473 to a03090b Compare September 26, 2025 14:38

github-actions bot added the arrow Changes to the arrow crate label Sep 26, 2025

one more link

03558a1

alamb approved these changes Sep 26, 2025

View reviewed changes

kylebarron approved these changes Sep 26, 2025

View reviewed changes

paleolimbot and others added 2 commits September 27, 2025 21:16

Update parquet-geospatial/src/bounding.rs

8e85bdf

Co-authored-by: Kyle Barron <kylebarron2@gmail.com>

fix suggestion

2cfa199

alamb merged commit b6bea12 into apache:main Sep 28, 2025
30 checks passed

paleolimbot mentioned this pull request Oct 1, 2025

Support writing GeospatialStatistics in Parquet writer #8523

Closed

alamb mentioned this pull request Oct 17, 2025

Add geospatial statistics creation support for GEOMETRY/GEOGRAPHY Parquet logical types #8411

Closed


		use crate::interval::{Interval, IntervalTrait, WraparoundInterval};

		/// Geometry bounder

Conversation

paleolimbot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

kylebarron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Sep 26, 2025

Uh oh!

kylebarron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Sep 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paleolimbot commented Sep 22, 2025 •

edited

Loading