feat: recognize geometry and geography types [1/3]

[wirybeaver]

Summary

Part 1 of the geospatial support stack. This PR adds type and table-metadata awareness only; geospatial data reads and writes are intentionally implemented by the following PRs.

• Adds Iceberg v3 geometry and geography primitive types with CRS and edge-interpolation metadata.
• Parses and serializes canonical Iceberg type strings while preserving the supplied CRS identifier.
• Places the detailed API under spec::geospatial and re-exports only GeometryType and GeographyType from spec.
• Marks geospatial schemas as v3-only and validates unsupported defaults, sort orders, and transforms.
• Allows Glue to publish informational geometry/geography column strings; Glue readers continue to use Iceberg metadata as the source of truth. The broader existing Glue fallback behavior is tracked in Glue schema conversion should not fail for display-only Iceberg types #3375.

Scope and boundaries

PR	Supported in that layer	Intentionally deferred
#2933 — table/type layer	Parse and serialize table metadata; schema evolution and v3 validation; preserve CRS/edge metadata; informational Glue column metadata	Arrow/Avro value decoding and Parquet I/O return explicit FeatureUnsupported errors so Geo values cannot be mistaken for ordinary binary
#3341 — read layer	Arrow WKB extension import/export, Avro/WKB byte decoding, CRS validation, and safe handling of scalar Parquet statistics during filtering	Parquet file writing
#3342 — write layer	Parquet geometry/geography logical types, CRS/edge metadata, omitted optional scalar bounds, and writer/reader round-trip coverage	Geospatial zonemap/bounding-box statistics and spatial predicates

The explicit unsupported branches in this PR are data-plane boundaries, not failures to recognize or preserve the types in Iceberg table metadata. They are replaced by working implementations in #3341 and #3342.

Stack

1. This PR: table and type interactions
2. feat: add geospatial read support [2/3] #3341: read path
3. feat: add geospatial parquet write support [3/3] #3342: Parquet write path

Tests

• cargo check -p iceberg --locked
• cargo check -p iceberg-catalog-glue --locked
• cargo test -p iceberg geospatial
• cargo test -p iceberg-catalog-hms test_schema_with_geospatial_type_is_unsupported
• cargo test -p iceberg-catalog-glue test_schema_with_geospatial_type

[brgr-s]

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]

@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]

Great!

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

[paleolimbot]

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]

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]

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]

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]

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

[dannycjones]

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]

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]

#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?

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

[wirybeaver]

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

[dannycjones]

I think this is looking in good shape, thanks for addressing the feedback @wirybeaver. The schema round trips tests look great.

Two things:

• I think we need to remove the canonicalization of the CRS values for now. Normalization to lower case or something is OK IMO, but the changing between CRSes we deem equivalent isn't for now. I'd address as a follow up once there's consensus.
• I think what's missing given this implements reads and writes are some tests actually checking we can round trip the data. This should be in iceberg-rust. I'd like to see Spark but I don't think we have anything like that today. I've been experimenting a bit, I may share something at a later point.

[dannycjones]

Two other things come to mind right now.

• We should not allow transactions that try to set a sort order containing a geo field.
• We should explicitly disallow initial_default and write_default values being set for geo types.

[dannycjones]

Here's an example of a test in parquet_writer.rs I drafted for testing some of this.

    /// Write geometry and geography columns through the Iceberg writer and read them back through
    /// the Iceberg reader, checking that both the WKB payloads and the geospatial type parameters
    /// (CRS, edge algorithm) survive the Iceberg -> Arrow -> Parquet -> Arrow -> Iceberg trip.
    #[tokio::test]
    async fn test_parquet_writer_geospatial_data_roundtrip() -> Result<()> {
        let temp_dir = TempDir::new().unwrap();
        let file_io = FileIO::new_with_fs();
        let location_gen = DefaultLocationGenerator::with_data_location(
            temp_dir.path().to_str().unwrap().to_string(),
        );
        let file_name_gen =
            DefaultFileNameGenerator::new("test".to_string(), None, DataFileFormat::Parquet);

        let geom_type = PrimitiveType::Geometry(GeometryType::default());
        let geog_type = PrimitiveType::Geography(
            GeographyType::new(
                Some("OGC:CRS84".to_string()),
                IcebergEdgeInterpolationAlgorithm::Karney,
            )
            .unwrap(),
        );
        let schema: SchemaRef = Arc::new(
            Schema::builder()
                .with_schema_id(1)
                .with_fields(vec![
                    NestedField::required(1, "geom", Type::Primitive(geom_type.clone())).into(),
                    NestedField::optional(2, "geog", Type::Primitive(geog_type.clone())).into(),
                ])
                .build()
                .unwrap(),
        );

        let arrow_schema: ArrowSchemaRef = Arc::new(schema_to_arrow_schema(&schema).unwrap());
        let geom_wkb: Vec<Vec<u8>> = vec![wkb_point_xy(1.0, 2.0), wkb_point_xy(3.0, 4.0)];
        let geog_wkb: Vec<Option<Vec<u8>>> = vec![Some(wkb_point_xy(5.0, 6.0)), None];

        let geom_array = Arc::new(LargeBinaryArray::from_iter_values(geom_wkb.iter())) as ArrayRef;
        let geog_array = Arc::new(LargeBinaryArray::from_iter(geog_wkb.iter())) as ArrayRef;
        let to_write =
            RecordBatch::try_new(arrow_schema.clone(), vec![geom_array, geog_array]).unwrap();

        let output_file = file_io.new_output(
            location_gen.generate_location(None, &file_name_gen.generate_file_name()),
        )?;
        let mut pw = ParquetWriterBuilder::new(WriterProperties::builder().build(), schema.clone())
            .build(output_file)
            .await?;
        pw.write(&to_write).await?;
        let data_file = pw
            .close()
            .await?
            .into_iter()
            .next()
            .unwrap()
            .content(DataContentType::Data)
            .partition(Struct::empty())
            .partition_spec_id(0)
            .build()
            .unwrap();
        assert_eq!(data_file.record_count(), 2);

        // The Iceberg schema recovered from the file's Arrow schema must carry the geospatial
        // parameters, not just fall back to plain binary.
        let input_file = file_io.new_input(data_file.file_path())?;
        let file_metadata = input_file.metadata().await?;
        let reader = input_file.reader().await?;
        let mut parquet_reader = ArrowFileReader::new(file_metadata, reader);
        let parquet_metadata = parquet_reader.get_metadata(None).await?;
        let file_arrow_schema = parquet_to_arrow_schema(
            parquet_metadata.file_metadata().schema_descr(),
            parquet_metadata.file_metadata().key_value_metadata(),
        )
        .unwrap();
        let recovered = arrow_schema_to_schema(&file_arrow_schema).unwrap();
        assert_eq!(
            recovered.field_by_id(1).unwrap().field_type.as_ref(),
            &Type::Primitive(geom_type)
        );
        assert_eq!(
            recovered.field_by_id(2).unwrap().field_type.as_ref(),
            &Type::Primitive(geog_type)
        );

        let reader = ArrowReaderBuilder::new(file_io, Runtime::current()).build();
        let task = FileScanTask::builder()
            .with_file_size_in_bytes(data_file.file_size_in_bytes())
            .with_start(0)
            .with_length(0)
            .with_data_file_path(data_file.file_path.clone())
            .with_data_file_format(DataFileFormat::Parquet)
            .with_schema(schema)
            .with_project_field_ids(vec![1, 2])
            .with_case_sensitive(false)
            .build()
            .unwrap();
        let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as FileScanTaskStream;
        let batches: Vec<RecordBatch> = reader
            .read(tasks)
            .unwrap()
            .stream()
            .try_collect()
            .await
            .unwrap();

        let read_back = concat_batches(batches[0].schema_ref(), &batches).unwrap();
        assert_eq!(read_back.num_rows(), 2);

        let read_geom = read_back
            .column(0)
            .as_any()
            .downcast_ref::<LargeBinaryArray>()
            .unwrap();
        assert_eq!(
            read_geom.iter().collect::<Vec<_>>(),
            geom_wkb
                .iter()
                .map(|wkb| Some(wkb.as_slice()))
                .collect::<Vec<_>>()
        );

        let read_geog = read_back
            .column(1)
            .as_any()
            .downcast_ref::<LargeBinaryArray>()
            .unwrap();
        assert_eq!(
            read_geog.iter().collect::<Vec<_>>(),
            geog_wkb
                .iter()
                .map(|wkb| wkb.as_deref())
                .collect::<Vec<_>>()
        );

        Ok(())
    }

[dannycjones]

Honestly, I'd give this suggestion again: we should break this PR down into 3. Table interactions (parsing a table with geo, allowing metadata updates), read path (scary, need to make sure we're getting filtering right, this is the first time we're reading bounds and stats that aren't scalar), write path (mostly fine because we can just skip optional stats and bounds).

I'd like to get committer input and we can make their life easier by keeping the scope of the changes limited.

[wirybeaver]

@dannycjones Thanks for your suggestion. Let me create stacking PRs this week.

[wirybeaver]

Split complete as requested:

• feat: recognize geometry and geography types [1/3] #2933 is now part 1/3: table/type interactions (fa95b817)
• feat: add geospatial read support [2/3] #3341 is part 2/3: read path (73105dba)
• feat: add geospatial parquet write support [3/3] #3342 is part 3/3: Parquet write path (715ca291)

I also addressed the remaining review items: preserved CRS identifiers without semantic canonicalization, rejected geo sort orders and unsupported WKT/catalog conversions, validated WKB storage and CRS metadata, made geo datums explicitly unordered, skipped scalar geo bounds/statistics, and added read plus writer/reader round-trip coverage. Each original thread has a commit/PR-specific reply.

[CTTY]

Thanks for working on this! I'm halfway on my first pass and so far so good!

One suggestion: we can leave TODO comments on placeholders that you plan to implement later so we can better differ from those should be failed by design

[CTTY]

LGTM! Thanks for working on this! Could you resolve the merge conflicts as well?

[paleolimbot]

Most of my comments apply to the second and third members of this stack where I'll take a look after this merges.

All my comments on this section of the initial PR are addressed...thank you!

[CTTY]

Looks like there is another minor conflict, could you help resolve it?

[wirybeaver]

@CTTY The Conflicts are resolved.