feat(vortex-spatial): preserve GeoArrow metadata

[HarukiMoriarty]

Rationale for this change

Vortex preserved GeoArrow crs metadata but dropped crs_type and edges, making Arrow round-trips lossy.

What changes are included in this PR?

• Add crs_type and edges to SpatialMetadata with backward-compatible protobuf encoding.
• Preserve all three fields through spatial Arrow import/export and WKB conversion.
• Move shared metadata and geometry helpers out of extension/mod.rs.
• Add metadata serialization and GeoArrow round-trip tests.

What APIs are changed? Are there any user-facing changes?

Exports CrsType and Edges. Spatial computation is unchanged; geo kernels remain planar.

[codspeed-hq]

Merging this PR will not alter performance

✅ 1962 untouched benchmarks
⏩ 89 skipped benchmarks¹

---

_{Comparing nemo/geo-metadata (1ec22c0) with develop (ca7f626)}

Footnotes

1. 89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[connortsui20]

might as well just add a constructor that takes crs?

[claude]

The file split is a good cut, and decodes_legacy_crs_only_bytes is exactly the right test to pin the wire format. Four structural problems though, all downstream of one choice.

SpatialMetadata makes illegal states representable

crs, crs_type and edges are three independent pub fields, so to_geoarrow has to reject combinations at conversion time: crs_type with no crs, Projjson over a non-object, AuthorityCode without a colon. from_geoarrow is infallible, and geoarrow's Crs is a bare #[derive(Deserialize)] with no validation, so those states enter through Arrow import and only fail on the way out. Verified against this branch:

crs="4326"      crs_type=authority_code   import=OK  export=Err(spatial: authority-code CRS must have the form AUTHORITY:CODE)
crs="EPSG:4326" crs_type=projjson         import=OK  export=Err(spatial: invalid PROJJSON CRS: expected value at line 1 column 1)

A GeoParquet file from a slightly off-spec producer reads, filters and projects fine, and then every to_arrow_field, DuckDB export and to_arrow on it fails. One field removes the class:

pub crs: Option<Crs>,  // Projjson(Value) | Wkt2_2019(String) | AuthorityCode { authority, code } | Srid(String) | Unknown(String)

That makes all three vortex_ensure!s unreachable and to_geoarrow infallible. If that's too much churn for this PR, the shallower fix is ExtVTable::validate_dtype, which already says it "should check both storage dtype and extension metadata" and runs on every construction path including file deserialization. All eight spatial impls check storage only today.

The failure now reaches query execution

Threading real metadata into point_geometries and friends means a bad CRS breaks ST_Area, ST_Intersects and ST_Distance, not just export. geo_types::Geometry carries no CRS, so the value is built, validated, and discarded. The PR deletes the comment that recorded why this was deliberate ("CRS does not affect planar geometry ops, so default metadata is used") without saying what replaced the reasoning.

to_wkb is the same story with no upside: geoarrow_to_wkb passes the metadata to cast, then to_array_ref() drops it (geoarrow-array documents that method as omitting all spatial extension information), so Arc::clone(data_type.metadata()) has no observable effect on the exported column.

PROJJSON stored as text does not round-trip

from_geoarrow keeps Value::to_string(), so an Arrow hop rewrites the CRS with sorted keys and no whitespace. SpatialMetadata derives Eq, so the DType changes under it:

before:  Geometry(crs={\n  "type": "GeographicCRS",\n  "name": "WGS 84"\n})
after:   Geometry(crs={"name":"WGS 84","type":"GeographicCRS"})
dtype == roundtripped  ->  false

That breaks schema matching for anything that compares dtypes across an Arrow boundary. roundtrips_geoarrow_metadata misses it because it compares against projjson.to_string(), which is already normalized. Storing the parsed Value rather than text fixes it.

An unknown enum value makes the file unreadable

CrsType::try_from / Edges::try_from in TryFrom<SpatialMetadataProto> turn an unrecognized discriminant into an error, and deserialize_metadata runs when the ExtDType is materialized, so one future crs_type value fails the whole scan rather than the metadata. [0x10, 0x09] gives spatial: invalid CRS type: unknown enumeration value 9. An unknown proto field on the same message is skipped silently, so the field-level story is forward compatible and the enum-level story is not. Mapping the unknown value to None keeps the CRS string readable. Both enums also want #[non_exhaustive], since they're public and adding a variant is otherwise a semver break.

Smaller things

• make_line_metadata still matches on (&left.crs, &right.crs). ST_MakeLine over two spherical-edge operands falls into the (None, None) arm and returns SpatialMetadata::default(), so the output linestring claims planar edges. Two operands with the same CRS string but different crs_type are also accepted as compatible.
• Nothing reads edges. The kernels stay planar, so the PR records a claim and then ignores it. Rejecting or warning on non-planar edges in the length, area and distance kernels would at least make the gap visible.
• Display still prints only the CRS, so a spherical and a planar dtype render identically. ChunkedArray chunk dtype {} does not match outer dtype {} will print the same string twice.
• Edges::default() is Andoyer, not planar, because prost::Enumeration derives Default from the first variant. That contradicts the doc one line above it.
• The DuckDB boundary carries crs only (convert/dtype.rs:270 out, :176 in), so a round trip drops crs_type and edges and downgrades PROJJSON to an opaque string. Worth a test either way, so the loss is a decision rather than an accident.
• Seventeen ..Default::default() sites and no SpatialMetadata constructor. The next field added is silently defaulted at every one of them with no compiler nudge.
• flatten_row_offsets lost the statement that row_offsets has len + 1 entries and that row r is coordinates[row_offsets[r]..row_offsets[r + 1]]. envelope.rs:111 and length.rs:69 index row_offsets[r + 1] on that contract with no guard. native_geometry_scalar_from_wkb is public and no longer documents when it returns None, which vortex-duckdb's pushdown depends on. extension/mod.rs also has no //! doc now that it's a pure barrel.
• The five WKB tests in literal.rs share one body and lost their byte annotations in the move, so 4u32 and 5u32 are now bare literals. rstest cases plus a #[track_caller] assert helper would collapse them, the way metadata.rs already does in this same diff.

---

Generated by Claude Code

[connortsui20]

Structural review. The Arrow boundary work looks right. Three things underneath it need another pass.

The planar kernels became fallible on metadata they never read. The description says "Spatial computation is unchanged; geo kernels remain planar". The math is unchanged, the error behaviour is not. point_geometries and its siblings used to build their geoarrow type from SpatialMetadata::default(), and the doc comment this PR deletes said why: "CRS does not affect planar geometry ops, so default metadata is used." They now route the column's real metadata through to_geoarrow, which validates it. A Point column imported from {"crs":"4326","crs_type":"authority_code"} reads, filters and projects fine, then ST_Area fails with spatial: authority-code CRS must have the form AUTHORITY:CODE. The same call sits under PointData::to_wkb, which is the DuckDB GEOMETRY export path, so SELECT geom FROM read_vortex(...) errors mid-scan.

to_wkb cannot use the value either. geoarrow_to_wkb ends in ArrayRef::from_arrow(wkb.to_array_ref().as_ref(), false), and to_array_ref is documented to "omit any spatial extension information", so the metadata now threaded into it is discarded. That part of the change has no observable effect.

Validation sits on the wrong side of the boundary. from_geoarrow is infallible and to_geoarrow is fallible, so invalid metadata enters freely and only fails on the way out. geoarrow's Crs is a plain serde derive, so its validating constructors are bypassed and any producer can emit both of these:

field metadata	from_arrow_field	to_arrow_field
{"crs":"4326","crs_type":"authority_code"}	ok	spatial: authority-code CRS must have the form AUTHORITY:CODE
{"crs":"EPSG:4326","crs_type":"projjson"}	ok	spatial: invalid PROJJSON CRS: expected value at line 1 column 1

Rejecting at import surfaces the problem while the producer can still be identified. The current shape produces a file that opens and then fails every export.

crs and crs_type want to be one field. Three of the six combinations are illegal, every field is pub, and the only check lives in to_geoarrow. An enum carrying the validated payload per representation makes the wrong states unrepresentable and moves the parse to one place:

pub enum Crs {
    Projjson(serde_json::Value),
    Wkt2_2019(String),
    AuthorityCode { authority: String, code: String },
    Srid(String),
    Unknown(String),
}

That also removes the ..Default::default() spread now on 17 struct literals, which is the shape that lets the next added field get silently defaulted at every site with no compiler nudge.

Smaller, same theme:

• make_line_metadata (make_line.rs:66) still matches on crs alone. Two spherical-edge operands hit the (None, None) arm and get SpatialMetadata::default() back, so the resulting line claims planar edges. ST_Envelope (envelope.rs:92) and GeometryAabb (aabb.rs:68, aabb.rs:119) emit default() as well, and the DuckDB conversion carries crs and nothing else in both directions. edges is preserved on import and dropped by every operation, so a downstream consumer cannot rely on it.
• An unrecognised discriminant fails the whole column. SpatialMetadata::deserialize(&[0x10, 0x09]) returns Err(spatial: invalid CRS type: unknown enumeration value 9), which propagates out of deserialize_metadata and takes the DType with it. Unknown field tags are skipped correctly, so only the enums are strict. GeoArrow has extended CrsType before. Keeping the raw i32 and treating an unknown value as an opaque encoding degrades instead of breaking.
• Display still prints only the CRS, so two dtypes that PartialEq reports as different render identically. A chunk mismatch reads chunk dtype vortex.st.wkb[Geometry(crs=EPSG:4326)](binary) does not match outer dtype vortex.st.wkb[Geometry(crs=EPSG:4326)](binary). A multi-line PROJJSON string also breaks the rendering outright.

Open question: should PROJJSON be stored parsed rather than as text? from_geoarrow stringifies the object and to_geoarrow parses it back, so an Arrow round trip reorders the keys and the DType stops comparing equal to itself. Storing serde_json::Value fixes the equality and drops a per-batch parse from the export path.

Doc and test regressions from the file split

• flatten_row_offsets (geometry.rs:78) lost the doc stating row_offsets.len() == storage.len() + 1 and coordinates[row_offsets[r]..row_offsets[r + 1]], plus the worked MultiPolygon offsets table. envelope.rs:110 and length.rs index row_offsets[r + 1] unguarded on that contract.
• native_geometry_scalar_from_wkb (literal.rs:42) is pub and its doc no longer says when the Option is None. vortex-duckdb/src/convert/expr.rs treats None as "do not push this predicate down".
• from_geoarrow (metadata.rs:203) kept value.as_str().map_or_else(|| value.to_string(), str::to_owned) but dropped the comment explaining why the branches are not equivalent. Collapsing it to value.to_string() would JSON-encode every string CRS.
• The five WKB tests in literal.rs lost every byte annotation (// geometry type: point, // one ring, // each member is a full WKB point), leaving bare 1u8 / 3u32 / 4u32 literals. They also share one body shape and repeat the same let ... else { panic!(...) } block five times, where #[rstest] cases and a #[track_caller] helper would do. metadata.rs in this same PR uses #[rstest] correctly for the same job.
• extension/mod.rs has no //! doc after being rewritten into a re-export barrel.

---

Generated by Claude Code