feat: support geometry and geography types by wirybeaver · Pull Request #2933 · apache/iceberg-rust

wirybeaver · 2026-07-31T02:10:14Z

Summary

This draft PR adds Iceberg Geometry/Geography primitive type support by reusing arrow-rs/parquet-geospatial support instead of introducing a local geospatial model.

Adds GeometryType and GeographyType with an Iceberg-owned EdgeInterpolationAlgorithm, translating to parquet_geospatial::WkbEdges at the Arrow boundary.
Converts Geometry/Geography to Arrow WKB extension metadata and enables Parquet geospatial logical type writing.
Maps Avro, Glue, and HMS representations to bytes/binary.
Rejects non-null JSON defaults, blocks unsupported partition transforms, and skips byte min/max statistics for spatial values.

Related issues

Related to #2411 and #1884.

Supersedes #2653, which was closed automatically by the stale bot. Existing review discussion remains available there.

brgr-s · 2026-07-31T13:31:23Z

In case you are interested, PR #2019 (CLOSED as stale) also has code related to geo types.

paleolimbot

There are a few details around the conversion of types, but this is looking good to me!

paleolimbot

I am not qualified to approve this from the icberg-rust end of things, but the geography type conversion seems solid (I suggested some optional improvements to errors inline). Thank you!

Happy to review any follow-ups on the statistics/pruning end of things if you are interested (or to attempt them if you aren't!).

I kept arrow-rs WkbType rather than introducing an Iceberg-specific copy because parquet 58.4 consumes its own WkbType; replacing only the Iceberg Arrow type would not fix the pre-59.1 Geography behavior and could make Parquet logical-type writing incompatible.

I think this is fine, although anybody who wants to write Geography will have to write invalid metadata into Iceberg (and will receive invalid metadata that will be rejected when reading using the arrow reader).

The workaround for this is to rewrite the metadata just for the Parquet reader, but I get how it's not worth adding that here since it will automatically resolve when arrow-rs is bumped. Our workaround for the write side is here:

https://github.com/apache/sedona-db/blob/ad83ab697b43998a4546359257015df71e210f18/rust/sedona-geoparquet/src/writer.rs#L681-L718

Can this be documented somehow?

wirybeaver · 2026-08-05T16:22:26Z

@paleolimbot Thanks for calling this out. I documented the arrow-rs 58.x Geography metadata limitation in 9a5d028 on the public Arrow schema conversion APIs. The docs now explain both directions: standards-compliant GeoArrow edges metadata may be interpreted as Geometry when importing, while exported Geography metadata uses the legacy algorithm key for parquet-rs 58.x compatibility and may be rejected or misinterpreted by standards-compliant Arrow readers. The note also links to apache/arrow-rs#10065, where this is fixed for 59.1.0+.

paleolimbot · 2026-08-05T16:28:50Z

Great!

Can this be unmarked as draft? (i.e., is it ready for review from an iceberg committer?)

paleolimbot · 2026-08-06T15:42:48Z

It seems like there's a merge conflict here. When that's fixed and CI is clean I'm happy to go fishing for iceberg-rust committers for review since this is a huge feature for us that will unblock a lot of things!

Kurtiscwright · 2026-08-12T20:57:32Z

I kept arrow-rs WkbType rather than introducing an Iceberg-specific copy because parquet 58.4 consumes its own WkbType; replacing only the Iceberg Arrow type would not fix the pre-59.1 Geography behavior and could make Parquet logical-type writing incompatible.

@paleolimbot @wirybeaver I am have a very high level understanding of the Geo data type details, but this comment in paleolimbot's review confuses me. Does this mean that as this PR currently stands if a Java Iceberg reader attempts to read a Geo type data file created by this Rust library the Reader will break?

Is the current implementation inter-mixable with Iceberg Reader & Writer implementations from the Java & Golang communities?

paleolimbot · 2026-08-13T01:58:15Z

Does this mean that as this PR currently stands if a Java Iceberg reader attempts to read a Geo type data file created by this Rust library the Reader will break?

Is the current implementation inter-mixable with Iceberg Reader & Writer implementations from the Java & Golang communities?

The reader will not break; however, until parquet is updated to 59.1.0, the conversion to and from Arrow types will have some errors. You can see which errors in the PR that fixed them ( apache/arrow-rs#10065 )...some of the cases are common, unfortunately. After the update to 59.1.0, these conversion to and from Arrow types will be the same as in golang and C++ (I don't believe Java converts to and from Arrow types, so there's nothing to compare to there).

We can inline that fix here or we can merge as is and get started on the rest of the geospatial support with a disclaimer. I don't mind which one (I'm happy to inline the fix as a PR into this one if that's helpful).

CTTY · 2026-08-17T23:44:35Z

DataFusion 55.0 is about to be released pretty soon (voting thread). I think we should just wait for datafusion 55.0 and arrow upgrade

paleolimbot · 2026-08-31T19:48:48Z

Just linking #2958, as that should have the requisite arrow-rs version.

dannycjones · 2026-09-02T19:29:03Z

I'd like to see this PR get some more attention and ultimately have support for the Geo types in iceberg-rust. Thank you for working on this, and for everyone who have provided reviews so far! With that said, the PR looks very large at the moment - can we find good ways to break it down and incrementally merge?

Variant so far has had success splitting into these streams: add ability to recognize the Variant type in tables and gracefully reject it / work around it (#2188), add the simpler unshredded read path for Variant (#2880), following up with shredded read and then writes.

@EwoutH shared some information previously which might be useful for breaking this down. #1884 (comment)

I'd like to take a look at this PR soon, but I think a good first PR to get merged in would be identifying the type and gracefully rejecting. In the meantime, after we cut 0.11.x it should allow #2958 to be merged soon after.

paleolimbot · 2026-09-02T19:53:41Z

can we find good ways to break it down and incrementally merge

Given that this requires some domain expertise to review (e.g., from the spatial side), I'd prefer to have this as a unit so that I can keep track of spatial correctness.

wirybeaver · 2026-09-03T22:31:00Z

#2598 get merged. I am working on the fix

dannycjones

Thanks again work working on this, @wirybeaver! I've taken a look, although I apologize if any comments don't make sense - I'm new to geospatial types.

I don't see any read tests - is that left intentionally out of scope?

dannycjones · 2026-09-04T13:08:06Z

+        assert!(data_file.lower_bounds().is_empty());
+        assert!(data_file.upper_bounds().is_empty());


Can you include an assertion message about why the bounds are empty? Is it simply because the field is optional, and this is left as a follow-up?

I see its a follow-up, a code comment to confirm that would be good. #2933 (comment)

dannycjones · 2026-09-04T13:43:03Z

+fn edge_interpolation_algorithm_to_wkb_edges(algorithm: EdgeInterpolationAlgorithm) -> WkbEdges {
+    match algorithm {
+        EdgeInterpolationAlgorithm::Spherical => WkbEdges::Spherical,
+        EdgeInterpolationAlgorithm::Vincenty => WkbEdges::Vincenty,
+        EdgeInterpolationAlgorithm::Thomas => WkbEdges::Thomas,
+        EdgeInterpolationAlgorithm::Andoyer => WkbEdges::Andoyer,
+        EdgeInterpolationAlgorithm::Karney => WkbEdges::Karney,
+    }
+}


Why not impl From<EdgeInterpolationAlgorithm> for WkbEdges? That way we get idiomatic/easy translation from the iceberg-rust type into the Parquet type (and the same for any future file format equivalent).

Same in reverse: impl From<WkbEdges> for EdgeInterpolationAlgorithm.

dannycjones · 2026-09-04T16:55:23Z

+            PrimitiveType::Geometry(geometry) => match geometry.crs() {
+                Some(crs) => write!(f, "geometry({crs})"),
+                None => write!(f, "geometry"),
+            },


I don't see a variant in the spec without CRS provided. Should we write the default?

https://iceberg.apache.org/spec/#schemas

I know at least Spark rejects plain GEOMETRY but I don't know the relationship to iceberg.

dannycjones · 2026-09-04T17:00:55Z

+                (
+                    PrimitiveType::Geometry(_) | PrimitiveType::Geography(_),
+                    JsonValue::String(_),
+                ) => Err(Error::new(
+                    ErrorKind::DataInvalid,
+                    "Geometry and geography defaults must be null",
+                )),


This seems wrong. While we may not allow defaults, there is a valid deserialization from JSON in the spec.

We need to add the check elsewhere in the code to disallow defaults.

dannycjones · 2026-09-04T17:08:30Z

+                    wkb_edges_to_edge_interpolation_algorithm(
+                        wkb_type.metadata().algorithm.unwrap_or_default(),
+                    ),


Given this is the Parquet default, I'd rather we explicitly default for Iceberg.

Suggested change

wkb_edges_to_edge_interpolation_algorithm(

wkb_type.metadata().algorithm.unwrap_or_default(),

),

wkb_edges_to_edge_interpolation_algorithm(

wkb_type.metadata().algorithm.unwrap_or(WkbEdges::Spherical),

),

dannycjones · 2026-09-04T17:14:53Z

        for i in 0..fields.len() {
            let field = &fields[i];
-            let field_type = &field_results[i];
+            let field_type = self.apply_field_extension_type(field, &field_results[i])?;


(sorry, this comment isn't super actionable)

I need to better understand Variant's approach here. It appears to attach the field extension type earlier (I think because of field ID shenanigans), and I'm wondering if we should align or not. If we choose not to align, I think a comment is necessary on why they are separate.

dannycjones · 2026-09-04T17:17:31Z

+            Type::Variant(_) => {
+                // A variant column's storage is a struct; tag the field with the canonical
+                // `arrow.parquet.variant` extension type so consumers read it as a Variant, not a struct.
+                arrow_field = arrow_field.with_extension_type(VariantExtensionType);
+            }


I think we should update this to be try_with_extension_type for consistency (or update geospatial types to use with_extension_type if justified).

paleolimbot

Just a look from the spatial end...I think the translations for None CRSes aren't quite correct (this was one of the errors that was fixed by the upstream parquet-geospatial change). Thank you!

Co-authored-by: Dewey Dunnington <10995762+paleolimbot@users.noreply.github.com>

wirybeaver · 2026-09-05T23:13:39Z

Add @paleolimbot as the co-author for the substantial review. Will check other folks review later on.

wirybeaver mentioned this pull request Jul 31, 2026

Support geo types #1884

Open

This was referenced Jul 31, 2026

feat: support geometry and geography types #2653

Closed

EPIC: v3 Support Tracking #2411

Open

paleolimbot reviewed Aug 2, 2026

View reviewed changes

paleolimbot approved these changes Aug 3, 2026

View reviewed changes

wirybeaver marked this pull request as ready for review August 6, 2026 01:50

wirybeaver force-pushed the support-geometry-geography-types branch from 9a5d028 to 3ae7041 Compare August 6, 2026 02:24

wirybeaver force-pushed the support-geometry-geography-types branch from 3ae7041 to bb2e601 Compare August 9, 2026 01:42

wirybeaver force-pushed the support-geometry-geography-types branch from bb2e601 to c1d5762 Compare September 3, 2026 23:06

wirybeaver added 8 commits September 4, 2026 14:48

feat: support geometry and geography types

9464b82

fix: decouple geography edge algorithm API

ec9d75f

fix: normalize geospatial CRS metadata

355d07b

fix: validate geospatial CRS syntax

b6ed928

docs: note geography metadata compatibility

aa5838a

fix: avoid duplicate large binary match arm

65ef414

fix: update geospatial support for parquet 59.2

7d18d6d

refactor: simplify geospatial validation and tests

5951359

wirybeaver force-pushed the support-geometry-geography-types branch from 3f3a885 to 5951359 Compare September 4, 2026 16:04

dannycjones suggested changes Sep 4, 2026

View reviewed changes

paleolimbot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/iceberg/src/arrow/schema.rs

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs

Comment thread crates/iceberg/src/arrow/schema.rs Outdated

Comment thread crates/iceberg/src/arrow/schema.rs

Comment thread crates/iceberg/src/arrow/schema.rs

fix: preserve geospatial CRS semantics

dd55c89

Co-authored-by: Dewey Dunnington <10995762+paleolimbot@users.noreply.github.com>

		assert!(data_file.lower_bounds().is_empty());
		assert!(data_file.upper_bounds().is_empty());

Uh oh!

Conversation

wirybeaver commented Jul 31, 2026

Summary

Related issues

Uh oh!

brgr-s commented Jul 31, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wirybeaver commented Aug 5, 2026

Uh oh!

paleolimbot commented Aug 5, 2026

Uh oh!

paleolimbot commented Aug 6, 2026

Uh oh!

Kurtiscwright commented Aug 12, 2026

Uh oh!

paleolimbot commented Aug 13, 2026

Uh oh!

CTTY commented Aug 17, 2026

Uh oh!

paleolimbot commented Aug 31, 2026

Uh oh!

dannycjones commented Sep 2, 2026

Uh oh!

paleolimbot commented Sep 2, 2026

Uh oh!

wirybeaver commented Sep 3, 2026

Uh oh!

dannycjones 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wirybeaver commented Sep 5, 2026

Uh oh!