james-willis opened a new pull request, #1129: URL: https://github.com/apache/sedona-db/pull/1129
## What Adds an open-ended `Extension(Arc<dyn SedonaExtensionType>)` variant to the core `SedonaType` enum, so a new logical type can flow through function dispatch, coercion, display, and equality without a core-crate change per new type — only for this one slot, added once. This is a **prototype/spike**, opened as draft for early visibility, not requesting review yet. No docs, no CHANGELOG entry — treat the API as unstable and likely to change shape before (if) it's proposed for real. ## Context Came out of a design-doc comment (paleolimbot) on an internal Tensor-type proposal: the existing "adding a variant is a closed-enum core-crate change" framing was called out as a pragmatic historical choice, not a design constraint, with two suggested alternatives — a Struct detected via `TypeMatcher` (no core change), or an open-ended `Extension` variant (a real but likely small one-time change). This PR is the second path, prototyped to see how it actually holds up against the real codebase rather than staying theoretical. ## What's in it - `SedonaExtensionType` trait (new `extension_type_registry.rs`): `storage_type`, `extension_name`, `logical_type_name`, `extension_metadata`, equality via `as_any()`/`dyn_eq()`. Implementing this is all the *forward* direction (build a value, match it in a signature, display it) needs — no registry involved. - The *backward* direction — recovering a `SedonaType` from a bare external `Field`'s Arrow extension metadata (e.g. reading a Parquet file back) — needs something more: `SedonaType::from_storage_field` is called from ~40 sites across nearly every crate in the workspace, many with no session/context object reachable at all, so the existing per-`SedonaContext` registry pattern (`RasterLoaderRegistry`) doesn't fit here. This uses a **process-global** registry instead (`register_extension_type` / `lookup_extension_type_factory`). Flagging this explicitly since it's a real architectural tradeoff, not a detail — open to other opinions on it. - Wired the new variant into every exhaustive match on `SedonaType` found by actually compiling the whole workspace (not just grepping): `storage_type`/`extension_name`/`extension_type`/`logical_type_name`/`match_signature`/`Display` in `sedona-schema`, plus two sites in `sedona-functions/sd_format.rs` and one in `sedona-geoarrow-c` — 9 sites across 3 crates total. - `ArgMatcher::is_extension(name)` in `matchers.rs`, with a test proving a two-argument signature (shaped like a hypothetical `TN_Add(Tensor, Tensor)`) matches and resolves a return type through a toy extension type, the same way `is_raster()`/`is_geometry()` do today for the built-in types. ## Verification - `cargo check --workspace` — every crate (including `sedona-raster-gdal`, the Python bindings, the R bindings) compiles clean. - `cargo test -p sedona-schema` (60 tests), `sd_format` (33 tests), `sedona-geoarrow-c` (14 tests) — all pass, zero regressions. - `cargo clippy -p sedona-schema --lib -- -D warnings` — clean. ## Not in this PR - Docs, CHANGELOG. - Any real consumer of `SedonaType::Extension` (e.g. an actual Tensor type) — this PR is only the mechanism. - A decision on global vs. session-scoped registry — flagged above as open, not resolved here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
