Add tests and fix corner cases for Parquet/GeoArrow extension type conversion by paleolimbot · Pull Request #10065 · apache/arrow-rs

paleolimbot · 2026-06-03T22:52:17Z

Which issue does this PR close?

Closes Parquet geospatial conversion uses metadata key "algorithm" instead of "edges" in geoarrow metadata #9929.

Rationale for this change

There were several issues with conversion identified when I tried to integrate this in SedonaDB and that came to light when the spec was recently clarified.

I am sorry for missing these changes when I reviewed the initial implementation.

What changes are included in this PR?

A Parquet crs of None for geometry or geography is now converted to a GeoArrow CRS of "OGC:CRS84" (the named value for the default CRS in the Parquet spec)
A Parquet crs of "srid:0" is now converted to a GeoArrow "omitted" CRS. This was recently clarified in the Parquet spec (srid:0 is a named example in the list of allowed values).
A GeoArrow missing CRS is now encoded as "srid:0"
A GeoArrow CRS that is "lonlat-like" is now encoded as a Parquet crs of None. This logic was included in the previous implementation but was reversed (Parquet CRSes that looked like lonlat were omitted when written to GeoArrow, which is not correct).
The GeoArrow metadata struct uses the name "algorithm" and was serializing it to JSON. The GeoArrow spec uses the "edges" key. This led to invalid metadata being generated which was either rejected or incorrectly interpreted by consumers.

Are these changes tested?

Yes. I added high-level end-to-end LogicalType <-> extension metadata tests, since that is what matters (there were a few lower level tests that I updated as well).

paleolimbot · 2026-06-05T02:06:14Z

+            // Geometry with default CRS (defaults to OGC:CRS84 per Parquet spec)
+            (LogicalType::geometry(None), r#"{"crs":"OGC:CRS84"}"#),
+            // Geometry with srid:0 should result in an unset (omitted) CRS
+            (LogicalType::geometry(Some("srid:0".to_string())), r#"{}"#),
+            // Geometry with custom CRSes (authority:code and partial projjson)
+            (
+                LogicalType::geometry(Some("EPSG:4267".to_string())),
+                r#"{"crs":"EPSG:4267"}"#,
+            ),
+            (
+                LogicalType::geometry(Some(
+                    r#"{"id":{"authority":"EPSG","code":4326}}"#.to_string(),
+                )),
+                r#"{"crs":{"id":{"authority":"EPSG","code":4326}}}"#,
+            ),
+            // Geography with default CRS (default OGC:CRS84, spherical edges)
+            (
+                LogicalType::geography(None, None),
+                r#"{"crs":"OGC:CRS84","edges":"spherical"}"#,
+            ),
+            // Geography with explicit edges
+            (
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::SPHERICAL)),
+                r#"{"crs":"OGC:CRS84","edges":"spherical"}"#,
+            ),
+            (
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::KARNEY)),
+                r#"{"crs":"OGC:CRS84","edges":"karney"}"#,
+            ),
+            (
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::VINCENTY)),
+                r#"{"crs":"OGC:CRS84","edges":"vincenty"}"#,
+            ),
+            (
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::ANDOYER)),
+                r#"{"crs":"OGC:CRS84","edges":"andoyer"}"#,
+            ),
+            (
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::THOMAS)),
+                r#"{"crs":"OGC:CRS84","edges":"thomas"}"#,
+            ),
+            // Geometry with srid:0 should result in an unset (omitted) CRS
+            // and spherical edges
+            (
+                LogicalType::geography(Some("srid:0".to_string()), None),
+                r#"{"edges":"spherical"}"#,
+            ),
+            // Geography with custom CRSes (authority:code and partial projjson)
+            (
+                LogicalType::geography(Some("EPSG:4267".to_string()), None),
+                r#"{"crs":"EPSG:4267","edges":"spherical"}"#,
+            ),
+            (
+                LogicalType::geography(
+                    Some(r#"{"id":{"authority":"EPSG","code":4326}}"#.to_string()),
+                    None,
+                ),
+                r#"{"crs":{"id":{"authority":"EPSG","code":4326}},"edges":"spherical"}"#,
+            ),


These are the test cases for reading Parquet files. For GeoArrow implementation to correctly recognize the intent of the CRS field in the Parquet file, this is the GeoArrow extension metadata that must be produced in each of these cases.

There were a few problems with the existing implementation:

LogicalType::geometry(None) (i.e., Parquet default CRS) was read as as {} (which in GeoArrow land means "I don't know what the CRS is)

LogicalType::geography(Some(...), None) (i.e. Parquet default edge algorithm) was read as {"crs":"..."} (i.e., no edges in the GeoArrow metadata, which consumers would recognize as GEOMETRY and not GEOGRAPHY)

LogicalType::geography(Some(...), Some(EdgeInterpolationAlgorithm::SPHERICAL)) (i.e. an actual edge algorithm) was read as {"crs":"...","algorithm":"spherical"}. The "algorithm" key isn't valid in GeoArrow and this would be rejected by consumers or ignored and read as GEOMETRY (not geography).

paleolimbot · 2026-06-05T02:09:40Z

+            // Geometry with no CRS should be GEOMETRY(srid:0)
+            (r#"{}"#, LogicalType::geometry(Some("srid:0".to_string()))),
+            // Geometry with string CRS
+            (
+                r#"{"crs":"EPSG:4267"}"#,
+                LogicalType::geometry(Some("\"EPSG:4267\"".to_string())),
+            ),
+            // Geometry with PROJJSON CRS
+            (
+                r#"{"crs":{"id":{"authority":"EPSG","code":3857}}}"#,
+                LogicalType::geometry(Some(
+                    r#"{"id":{"authority":"EPSG","code":3857}}"#.to_string(),
+                )),
+            ),
+            // Geometry with lon/lat CRSes (canonically removed because lon/lat is the
+            // default Parquet CRS)
+            (r#"{"crs":"OGC:CRS84"}"#, LogicalType::geometry(None)),
+            (r#"{"crs":"EPSG:4326"}"#, LogicalType::geometry(None)),
+            (
+                r#"{"crs":{"id":{"authority":"EPSG","code":4326}}}"#,
+                LogicalType::geometry(None),
+            ),
+            (
+                r#"{"crs":{"id":{"authority":"EPSG","code":"4326"}}}"#,
+                LogicalType::geometry(None),
+            ),
+            (
+                r#"{"crs":{"id":{"authority":"OGC","code":"CRS84"}}}"#,
+                LogicalType::geometry(None),
+            ),
+            // Geography with no CRS, spherical edges
+            (
+                r#"{"edges":"spherical"}"#,
+                LogicalType::geography(Some("srid:0".to_string()), None),
+            ),
+            // Geography with OGC:CRS84 and spherical edges
+            (
+                r#"{"crs":"OGC:CRS84","edges":"spherical"}"#,
+                LogicalType::geography(None, None),
+            ),
+            // Geography with different edge algorithms
+            (
+                r#"{"crs":"OGC:CRS84","edges":"karney"}"#,
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::KARNEY)),
+            ),
+            (
+                r#"{"crs":"OGC:CRS84","edges":"vincenty"}"#,
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::VINCENTY)),
+            ),
+            (
+                r#"{"crs":"OGC:CRS84","edges":"andoyer"}"#,
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::ANDOYER)),
+            ),
+            (
+                r#"{"crs":"OGC:CRS84","edges":"thomas"}"#,
+                LogicalType::geography(None, Some(EdgeInterpolationAlgorithm::THOMAS)),
+            ),
+            // Geography with custom CRS and edges
+            (
+                r#"{"crs":"EPSG:4267","edges":"karney"}"#,
+                LogicalType::geography(
+                    Some("\"EPSG:4267\"".to_string()),
+                    Some(EdgeInterpolationAlgorithm::KARNEY),
+                ),
+            ),
+            // Geography with PROJJSON CRS
+            (
+                r#"{"crs":{"id":{"authority":"EPSG","code":4267}},"edges":"spherical"}"#,
+                LogicalType::geography(
+                    Some(r#"{"id":{"authority":"EPSG","code":4267}}"#.to_string()),
+                    None,
+                ),
+            ),


These are the tests for writing. The main issue with writing was that "edges":"spherical", which GeoArrow uses to communicate GEOGRAPHY, was not actually writing a geography type because the implementation expected "algorithm". An unset CRS (geoarrow metadata {}) was also written as a Parquet default CRS which is technically incorrect but also not common.

paleolimbot · 2026-06-05T02:18:51Z

cc @BlakeOrth

BlakeOrth · 2026-06-05T16:28:10Z

Thanks for the ping, I'll make some time to review this today.

BlakeOrth

The changes here look good to me, I didn't find anything meaningful that would require any follow-up. Thanks for taking care of this!

alamb

Thank you @paleolimbot This all looks reasonable to me, but I am not enough of an expert to understand what the downstream implications of this are

@kylebarron I wonder if you might have some time to help review this PR and lend your specialized geoarrow knowledge ?

@paleolimbot do you have any sense

Is this a "breaking change" (as in could we put it out in a minor version or should we wait for the next major version)?
DO you have any suggestions on how to review this PR? Like other implementations I can cross reference with or links to the spec, etc?

alamb · 2026-06-08T20:29:13Z

+            // Geometry with string CRS
+            (
+                r#"{"crs":"EPSG:4267"}"#,
+                LogicalType::geometry(Some("\"EPSG:4267\"".to_string())),


Is it right to have embedded quotes here? It seems strange (maybe it is because it is a JSON string and thus needs to be a valid JSON value)?

LogicalType::geometry(Some("EPSG:4267".to_string())),

The same thing can be seen below too

kylebarron · 2026-06-08T20:57:29Z

Sorry, I haven't been following the latest updates in the Parquet native geometry type spec 🥲

kylebarron

Overall it looks good but I didn't take a super close look

paleolimbot added 5 commits June 3, 2026 17:22

add tests

0d6713a

more tests

a9db382

this is ok

cebc852

generated fix

5c6f6fc

fmt

f8af1c7

github-actions Bot added the parquet Changes to the parquet crate label Jun 3, 2026

paleolimbot commented Jun 5, 2026

View reviewed changes

paleolimbot marked this pull request as ready for review June 5, 2026 02:18

BlakeOrth approved these changes Jun 5, 2026

View reviewed changes

paleolimbot mentioned this pull request Jun 8, 2026

feat(table): WKB encoding + GeoArrow conversion for geometry/geography apache/iceberg-go#1138

Open

alamb reviewed Jun 8, 2026

View reviewed changes

kylebarron reviewed Jun 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests and fix corner cases for Parquet/GeoArrow extension type conversion#10065

Add tests and fix corner cases for Parquet/GeoArrow extension type conversion#10065
paleolimbot wants to merge 5 commits into
apache:mainfrom
paleolimbot:parquet-geospatial-typo

paleolimbot commented Jun 3, 2026 •

edited

Loading

Uh oh!

paleolimbot Jun 5, 2026

Uh oh!

paleolimbot Jun 5, 2026

Uh oh!

paleolimbot commented Jun 5, 2026

Uh oh!

BlakeOrth commented Jun 5, 2026

Uh oh!

BlakeOrth left a comment

Uh oh!

alamb left a comment

Uh oh!

alamb Jun 8, 2026

Uh oh!

kylebarron commented Jun 8, 2026

Uh oh!

kylebarron left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

paleolimbot commented Jun 3, 2026 • 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?

Uh oh!

paleolimbot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot commented Jun 5, 2026

Uh oh!

BlakeOrth commented Jun 5, 2026

Uh oh!

BlakeOrth left a comment

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

kylebarron commented Jun 8, 2026

Uh oh!

kylebarron left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

paleolimbot commented Jun 3, 2026 •

edited

Loading