From eb1afe2efda4e7dbb1fb100fb20913795209a893 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 18:34:08 +0100 Subject: [PATCH 01/18] add crs parsing and printing --- r/sedonadb/R/000-wrappers.R | 18 ++++++- r/sedonadb/R/crs.R | 8 +++ r/sedonadb/R/dataframe.R | 43 ++++++++++++++++ r/sedonadb/src/init.c | 7 +++ r/sedonadb/src/rust/Cargo.toml | 1 + r/sedonadb/src/rust/api.h | 1 + r/sedonadb/src/rust/src/lib.rs | 55 ++++++++++++++++++++ r/sedonadb/tests/testthat/test-crs.R | 75 ++++++++++++++++++++++++++++ 8 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 r/sedonadb/R/crs.R create mode 100644 r/sedonadb/tests/testthat/test-crs.R diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index df1f61fd9..9740d1d67 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -55,8 +55,22 @@ NULL } -`configure_proj_shared` <- function(`shared_library_path` = NULL, `database_path` = NULL, `search_path` = NULL) { - invisible(.Call(savvy_configure_proj_shared__impl, `shared_library_path`, `database_path`, `search_path`)) +`configure_proj_shared` <- function( + `shared_library_path` = NULL, + `database_path` = NULL, + `search_path` = NULL +) { + invisible(.Call( + savvy_configure_proj_shared__impl, + `shared_library_path`, + `database_path`, + `search_path` + )) +} + + +`parse_crs_metadata` <- function(`crs_json`) { + .Call(savvy_parse_crs_metadata__impl, `crs_json`) } diff --git a/r/sedonadb/R/crs.R b/r/sedonadb/R/crs.R new file mode 100644 index 000000000..579019aab --- /dev/null +++ b/r/sedonadb/R/crs.R @@ -0,0 +1,8 @@ +#' Parse CRS from GeoArrow metadata +#' +#' @param crs_json A JSON string representing the CRS (PROJJSON or authority code) +#' @returns A list with components: authority_code (e.g., "EPSG:5070"), srid (integer), +#' @keywords internal +sd_parse_crs <- function(crs_json) { + parse_crs_metadata(crs_json) +} diff --git a/r/sedonadb/R/dataframe.R b/r/sedonadb/R/dataframe.R index deb2e976c..f575737e2 100644 --- a/r/sedonadb/R/dataframe.R +++ b/r/sedonadb/R/dataframe.R @@ -316,6 +316,49 @@ as_nanoarrow_array_stream.sedonadb_dataframe <- function(x, ..., schema = NULL) #' @export print.sedonadb_dataframe <- function(x, ..., width = NULL, n = NULL) { + # Print class header + schema <- nanoarrow::infer_nanoarrow_schema(x) + ncols <- length(schema$children) + + cat(sprintf("# A sedonadb_dataframe: ? x %d\n", ncols)) + + # Print geometry column info + # we just use sd_parse_crs() to extract CRS info from ARROW:extension:metadata + geo_col_info <- character() + for (col_name in names(schema$children)) { + child <- schema$children[[col_name]] + ext_name <- child$metadata[["ARROW:extension:name"]] + if (!is.null(ext_name) && grepl("^geoarrow\\.", ext_name)) { + ext_meta <- child$metadata[["ARROW:extension:metadata"]] + crs_info <- "" + if (!is.null(ext_meta)) { + parsed <- sd_parse_crs(ext_meta) + if (!is.null(parsed$authority_code)) { + crs_info <- sprintf(" (CRS: %s)", parsed$authority_code) + } else if (!is.null(parsed$srid)) { + crs_info <- sprintf(" (CRS: EPSG:%d)", parsed$srid) + } else if (!is.null(parsed$name)) { + crs_info <- sprintf(" (CRS: %s)", parsed$name) + } else { + crs_info <- " (CRS: available)" + } + } + geo_col_info <- c(geo_col_info, sprintf("%s%s", col_name, crs_info)) + } + } + + if (length(geo_col_info) > 0) { + if (is.null(width)) { + width <- getOption("width") + } + + geo_line <- sprintf("# Geometry: %s", paste(geo_col_info, collapse = ", ")) + if (nchar(geo_line) > width) { + geo_line <- paste0(substr(geo_line, 1, width - 3), "...") + } + cat(paste0(geo_line, "\n")) + } + if (isTRUE(getOption("sedonadb.interactive", TRUE))) { sd_preview(x, n = n, width = width) } else { diff --git a/r/sedonadb/src/init.c b/r/sedonadb/src/init.c index 8405b2cc4..14af68ec7 100644 --- a/r/sedonadb/src/init.c +++ b/r/sedonadb/src/init.c @@ -60,6 +60,11 @@ SEXP savvy_configure_proj_shared__impl(SEXP c_arg__shared_library_path, return handle_result(res); } +SEXP savvy_parse_crs_metadata__impl(SEXP c_arg__crs_json) { + SEXP res = savvy_parse_crs_metadata__ffi(c_arg__crs_json); + return handle_result(res); +} + SEXP savvy_init_r_runtime__impl(DllInfo *c_arg___dll_info) { SEXP res = savvy_init_r_runtime__ffi(c_arg___dll_info); return handle_result(res); @@ -214,6 +219,8 @@ static const R_CallMethodDef CallEntries[] = { (DL_FUNC)&savvy_configure_proj_shared__impl, 3}, {"savvy_init_r_runtime_interrupts__impl", (DL_FUNC)&savvy_init_r_runtime_interrupts__impl, 2}, + {"savvy_parse_crs_metadata__impl", (DL_FUNC)&savvy_parse_crs_metadata__impl, + 1}, {"savvy_sedonadb_adbc_init_func__impl", (DL_FUNC)&savvy_sedonadb_adbc_init_func__impl, 0}, {"savvy_InternalContext_data_frame_from_array_stream__impl", diff --git a/r/sedonadb/src/rust/Cargo.toml b/r/sedonadb/src/rust/Cargo.toml index 2ce10cfdf..5c6f6aa26 100644 --- a/r/sedonadb/src/rust/Cargo.toml +++ b/r/sedonadb/src/rust/Cargo.toml @@ -39,5 +39,6 @@ sedona-expr = { workspace = true } sedona-geoparquet = { workspace = true } sedona-proj = { workspace = true } sedona-schema = { workspace = true } +serde_json = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } diff --git a/r/sedonadb/src/rust/api.h b/r/sedonadb/src/rust/api.h index 201039e14..0619cf0a9 100644 --- a/r/sedonadb/src/rust/api.h +++ b/r/sedonadb/src/rust/api.h @@ -21,6 +21,7 @@ SEXP savvy_configure_proj_shared__ffi(SEXP c_arg__shared_library_path, SEXP savvy_init_r_runtime__ffi(DllInfo *c_arg___dll_info); SEXP savvy_init_r_runtime_interrupts__ffi(SEXP c_arg__interrupts_call, SEXP c_arg__pkg_env); +SEXP savvy_parse_crs_metadata__ffi(SEXP c_arg__crs_json); SEXP savvy_sedonadb_adbc_init_func__ffi(void); // methods and associated functions for InternalContext diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 07c6f311a..4438ab3c0 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -66,3 +66,58 @@ fn configure_proj_shared( configure_global_proj_engine(builder)?; Ok(()) } + +#[savvy] +fn parse_crs_metadata(crs_json: &str) -> savvy::Result { + use sedona_schema::crs::deserialize_crs_from_obj; + + // The input is GeoArrow extension metadata, which is a JSON object like: + // {"crs": } + // We need to extract the "crs" field first. + let metadata: serde_json::Value = serde_json::from_str(crs_json) + .map_err(|e| savvy::Error::new(format!("Failed to parse metadata JSON: {e}")))?; + + let crs_value = metadata.get("crs"); + if crs_value.is_none() || crs_value.unwrap().is_null() { + return Ok(savvy::NullSexp.into()); + } + + let crs = deserialize_crs_from_obj(crs_value.unwrap())?; + match crs { + Some(crs_obj) => { + let auth_code = crs_obj.to_authority_code().ok().flatten(); + let srid = crs_obj.srid().ok().flatten(); + let name = crs_value.unwrap().get("name").and_then(|v| v.as_str()); + let proj_string = crs_obj.to_crs_string(); + + let mut out = savvy::OwnedListSexp::new(4, true)?; + out.set_name(0, "authority_code")?; + out.set_name(1, "srid")?; + out.set_name(2, "name")?; + out.set_name(3, "proj_string")?; + + if let Some(auth_code) = auth_code { + out.set_value(0, savvy::Sexp::try_from(auth_code.as_str())?)?; + } else { + out.set_value(0, savvy::NullSexp)?; + } + + if let Some(srid) = srid { + out.set_value(1, savvy::Sexp::try_from(srid as i32)?)?; + } else { + out.set_value(1, savvy::NullSexp)?; + } + + if let Some(name) = name { + out.set_value(2, savvy::Sexp::try_from(name)?)?; + } else { + out.set_value(2, savvy::NullSexp)?; + } + + out.set_value(3, savvy::Sexp::try_from(proj_string.as_str())?)?; + + Ok(out.into()) + } + None => Ok(savvy::NullSexp.into()), + } +} diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R new file mode 100644 index 000000000..00be0f5b1 --- /dev/null +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +test_that("sd_parse_crs works for GeoArrow metadata with EPSG", { + meta <- '{"crs": {"id": {"authority": "EPSG", "code": 5070}, "name": "NAD83 / Conus Albers"}}' + parsed <- sedonadb:::sd_parse_crs(meta) + expect_identical(parsed$authority_code, "EPSG:5070") + expect_identical(parsed$srid, 5070L) + expect_identical(parsed$name, "NAD83 / Conus Albers") + # The proj_string is the *unwrapped* and *minified* PROJJSON content + expect_match(parsed$proj_string, '"authority":"EPSG"', fixed = TRUE) + expect_match(parsed$proj_string, '"code":5070', fixed = TRUE) +}) + +test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { + # A realistic example of a local engineering CRS that wouldn't have an EPSG code + meta <- '{ + "crs": { + "type": "EngineeringCRS", + "name": "Construction Site Local Grid", + "datum": { + "type": "EngineeringDatum", + "name": "Local Datum" + }, + "coordinate_system": { + "subtype": "Cartesian", + "axis": [ + {"name": "Northing", "abbreviation": "N", "direction": "north", "unit": "metre"}, + {"name": "Easting", "abbreviation": "E", "direction": "east", "unit": "metre"} + ] + } + } + }' + parsed <- sedonadb:::sd_parse_crs(meta) + expect_null(parsed$authority_code) + expect_null(parsed$srid) + expect_identical(parsed$name, "Construction Site Local Grid") + expect_true(!is.null(parsed$proj_string)) +}) + +test_that("sd_parse_crs returns NULL if crs field is missing", { + expect_null(sedonadb:::sd_parse_crs('{"something_else": 123}')) + expect_null(sedonadb:::sd_parse_crs('{}')) +}) + +test_that("sd_parse_crs handles invalid JSON gracefully", { + expect_error( + sedonadb:::sd_parse_crs('invalid json'), + "Failed to parse metadata JSON" + ) +}) + +test_that("sd_parse_crs works with plain strings if that's what's in 'crs'", { + meta <- '{"crs": "EPSG:4326"}' + parsed <- sedonadb:::sd_parse_crs(meta) + # Note: PROJ/sedona normalizes EPSG:4326 (lat/lon) to OGC:CRS84 (lon/lat) + # for consistent axis order in WKT/GeoJSON contexts. + expect_identical(parsed$authority_code, "OGC:CRS84") + expect_identical(parsed$srid, 4326L) + expect_true(!is.null(parsed$proj_string)) +}) From 90d78fa6d391efef2b41d3b10f3d01a5d508d540 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 19:57:50 +0100 Subject: [PATCH 02/18] Update r/sedonadb/R/crs.R Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- r/sedonadb/R/crs.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/r/sedonadb/R/crs.R b/r/sedonadb/R/crs.R index 579019aab..8fb70e1c2 100644 --- a/r/sedonadb/R/crs.R +++ b/r/sedonadb/R/crs.R @@ -2,6 +2,8 @@ #' #' @param crs_json A JSON string representing the CRS (PROJJSON or authority code) #' @returns A list with components: authority_code (e.g., "EPSG:5070"), srid (integer), +#' name (character string with a human-readable CRS name), and proj_string (character +#' string with the PROJ representation of the CRS). #' @keywords internal sd_parse_crs <- function(crs_json) { parse_crs_metadata(crs_json) From fc7c3757a95912140abd516bdbe4bb563f96e657 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 19:58:06 +0100 Subject: [PATCH 03/18] Update r/sedonadb/src/rust/src/lib.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- r/sedonadb/src/rust/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 4438ab3c0..c12fefe64 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -113,7 +113,6 @@ fn parse_crs_metadata(crs_json: &str) -> savvy::Result { } else { out.set_value(2, savvy::NullSexp)?; } - out.set_value(3, savvy::Sexp::try_from(proj_string.as_str())?)?; Ok(out.into()) From ee8fa5cff05f1f0744fe0a9dd07e50d941dcbfdd Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 19:59:29 +0100 Subject: [PATCH 04/18] Update r/sedonadb/R/dataframe.R Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- r/sedonadb/R/dataframe.R | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/r/sedonadb/R/dataframe.R b/r/sedonadb/R/dataframe.R index f575737e2..06d2847b4 100644 --- a/r/sedonadb/R/dataframe.R +++ b/r/sedonadb/R/dataframe.R @@ -332,8 +332,13 @@ print.sedonadb_dataframe <- function(x, ..., width = NULL, n = NULL) { ext_meta <- child$metadata[["ARROW:extension:metadata"]] crs_info <- "" if (!is.null(ext_meta)) { - parsed <- sd_parse_crs(ext_meta) - if (!is.null(parsed$authority_code)) { + parsed <- tryCatch( + sd_parse_crs(ext_meta), + error = function(e) NULL + ) + if (is.null(parsed)) { + crs_info <- " (CRS: parsing error)" + } else if (!is.null(parsed$authority_code)) { crs_info <- sprintf(" (CRS: %s)", parsed$authority_code) } else if (!is.null(parsed$srid)) { crs_info <- sprintf(" (CRS: EPSG:%d)", parsed$srid) From 7e616e43f0c645da6815d2efb68f5588dd2de550 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 20:04:21 +0100 Subject: [PATCH 05/18] add crs printing tests --- r/sedonadb/tests/testthat/test-crs.R | 70 ++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index 00be0f5b1..e00c8a13d 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -73,3 +73,73 @@ test_that("sd_parse_crs works with plain strings if that's what's in 'crs'", { expect_identical(parsed$srid, 4326L) expect_true(!is.null(parsed$proj_string)) }) + +# Tests for CRS display in print.sedonadb_dataframe (lines 325-360 of dataframe.R) + +test_that("print.sedonadb_dataframe shows CRS info for geometry column with EPSG", { + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") + output <- capture.output(print(df, n = 0)) + + # Check that the Geometry line is present + + geo_line <- grep("^# Geometry:", output, value = TRUE) + expect_length(geo_line, 1) + + # Should show CRS information (OGC:CRS84 or EPSG:4326) + expect_match(geo_line, "geom .*(CRS: OGC:CRS84|CRS: EPSG:4326)") +}) + +test_that("print.sedonadb_dataframe shows CRS info with different SRID", { + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") + output <- capture.output(print(df, n = 0)) + + geo_line <- grep("^# Geometry:", output, value = TRUE) + expect_length(geo_line, 1) + expect_match(geo_line, "geom .*(CRS: EPSG:5070|CRS:.*5070)") +}) + +test_that("print.sedonadb_dataframe shows multiple geometry columns with CRS", { + df <- sd_sql( + " + SELECT + ST_SetSRID(ST_Point(1, 2), 4326) as geom1, + ST_SetSRID(ST_Point(3, 4), 5070) as geom2 + " + ) + output <- capture.output(print(df, n = 0)) + + geo_line <- grep("^# Geometry:", output, value = TRUE) + expect_length(geo_line, 1) + # Should contain both geometry columns + expect_match(geo_line, "geom1") + expect_match(geo_line, "geom2") +}) + +test_that("print.sedonadb_dataframe handles geometry without explicit CRS", { + # ST_Point without ST_SetSRID may not have CRS metadata + df <- sd_sql("SELECT ST_Point(1, 2) as geom") + output <- capture.output(print(df, n = 0)) + + # May or may not have a Geometry line depending on extension metadata + # At least it should not error + expect_true(any(grepl("sedonadb_dataframe", output))) +}) + +test_that("print.sedonadb_dataframe respects width parameter for geometry line", { + df <- sd_sql( + " + SELECT + ST_SetSRID(ST_Point(1, 2), 4326) as very_long_geometry_column_name_1, + ST_SetSRID(ST_Point(3, 4), 4326) as very_long_geometry_column_name_2 + " + ) + # Use a narrow width to trigger truncation + output <- capture.output(print(df, n = 0, width = 60)) + + geo_line <- grep("^# Geometry:", output, value = TRUE) + if (length(geo_line) > 0) { + # Line should be truncated with "..." + expect_lte(nchar(geo_line), 60) + expect_match(geo_line, "\\.\\.\\.$") + } +}) From ac60e14d335c1a75337c4e041eb734bef1d2d463 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 20:06:47 +0100 Subject: [PATCH 06/18] add license to crs.R --- r/sedonadb/R/crs.R | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/r/sedonadb/R/crs.R b/r/sedonadb/R/crs.R index 8fb70e1c2..8c3aa73b9 100644 --- a/r/sedonadb/R/crs.R +++ b/r/sedonadb/R/crs.R @@ -1,3 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + #' Parse CRS from GeoArrow metadata #' #' @param crs_json A JSON string representing the CRS (PROJJSON or authority code) From c6ee7917aee373c375a193b88be37c9e19a29b02 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Thu, 1 Jan 2026 20:52:36 +0100 Subject: [PATCH 07/18] address copilot review issues --- r/sedonadb/R/crs.R | 4 +- r/sedonadb/R/pkg-sf.R | 2 +- r/sedonadb/man/sd_parse_crs.Rd | 22 ++++++++ r/sedonadb/src/rust/src/lib.rs | 79 +++++++++++++++------------- r/sedonadb/tests/testthat/test-crs.R | 16 +++--- 5 files changed, 75 insertions(+), 48 deletions(-) create mode 100644 r/sedonadb/man/sd_parse_crs.Rd diff --git a/r/sedonadb/R/crs.R b/r/sedonadb/R/crs.R index 8c3aa73b9..f06ec9892 100644 --- a/r/sedonadb/R/crs.R +++ b/r/sedonadb/R/crs.R @@ -20,7 +20,9 @@ #' @param crs_json A JSON string representing the CRS (PROJJSON or authority code) #' @returns A list with components: authority_code (e.g., "EPSG:5070"), srid (integer), #' name (character string with a human-readable CRS name), and proj_string (character -#' string with the PROJ representation of the CRS). +#' string with the PROJ representation of the CRS), or \code{NULL} when no CRS +#' information is available, when the \code{"crs"} field is not present in the +#' metadata, or when parsing the CRS information fails. #' @keywords internal sd_parse_crs <- function(crs_json) { parse_crs_metadata(crs_json) diff --git a/r/sedonadb/R/pkg-sf.R b/r/sedonadb/R/pkg-sf.R index d40245702..32d12d11e 100644 --- a/r/sedonadb/R/pkg-sf.R +++ b/r/sedonadb/R/pkg-sf.R @@ -29,7 +29,7 @@ as_sedonadb_dataframe.sf <- function(x, ..., schema = NULL) { as_sedonadb_dataframe(new_sedonadb_dataframe(ctx, df), schema = schema) } -# dynamically registered in zzz.R +#' @exportS3Method sf::st_as_sf st_as_sf.sedonadb_dataframe <- function(x, ...) { stream <- nanoarrow::nanoarrow_allocate_array_stream() size <- x$df$collect(stream) diff --git a/r/sedonadb/man/sd_parse_crs.Rd b/r/sedonadb/man/sd_parse_crs.Rd new file mode 100644 index 000000000..e0419289b --- /dev/null +++ b/r/sedonadb/man/sd_parse_crs.Rd @@ -0,0 +1,22 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/crs.R +\name{sd_parse_crs} +\alias{sd_parse_crs} +\title{Parse CRS from GeoArrow metadata} +\usage{ +sd_parse_crs(crs_json) +} +\arguments{ +\item{crs_json}{A JSON string representing the CRS (PROJJSON or authority code)} +} +\value{ +A list with components: authority_code (e.g., "EPSG:5070"), srid (integer), +name (character string with a human-readable CRS name), and proj_string (character +string with the PROJ representation of the CRS), or \code{NULL} when no CRS +information is available, when the \code{"crs"} field is not present in the +metadata, or when parsing the CRS information fails. +} +\description{ +Parse CRS from GeoArrow metadata +} +\keyword{internal} diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index c12fefe64..840863fa2 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -77,46 +77,49 @@ fn parse_crs_metadata(crs_json: &str) -> savvy::Result { let metadata: serde_json::Value = serde_json::from_str(crs_json) .map_err(|e| savvy::Error::new(format!("Failed to parse metadata JSON: {e}")))?; - let crs_value = metadata.get("crs"); - if crs_value.is_none() || crs_value.unwrap().is_null() { - return Ok(savvy::NullSexp.into()); - } - - let crs = deserialize_crs_from_obj(crs_value.unwrap())?; - match crs { - Some(crs_obj) => { - let auth_code = crs_obj.to_authority_code().ok().flatten(); - let srid = crs_obj.srid().ok().flatten(); - let name = crs_value.unwrap().get("name").and_then(|v| v.as_str()); - let proj_string = crs_obj.to_crs_string(); - - let mut out = savvy::OwnedListSexp::new(4, true)?; - out.set_name(0, "authority_code")?; - out.set_name(1, "srid")?; - out.set_name(2, "name")?; - out.set_name(3, "proj_string")?; - - if let Some(auth_code) = auth_code { - out.set_value(0, savvy::Sexp::try_from(auth_code.as_str())?)?; - } else { - out.set_value(0, savvy::NullSexp)?; - } - - if let Some(srid) = srid { - out.set_value(1, savvy::Sexp::try_from(srid as i32)?)?; - } else { - out.set_value(1, savvy::NullSexp)?; - } + if let Some(crs_val) = metadata.get("crs") { + if crs_val.is_null() { + return Ok(savvy::NullSexp.into()); + } - if let Some(name) = name { - out.set_value(2, savvy::Sexp::try_from(name)?)?; - } else { - out.set_value(2, savvy::NullSexp)?; + let crs = deserialize_crs_from_obj(crs_val)?; + match crs { + Some(crs_obj) => { + let auth_code = crs_obj.to_authority_code().ok().flatten(); + let srid = crs_obj.srid().ok().flatten(); + let name = crs_val.get("name").and_then(|v| v.as_str()); + let proj_string = crs_obj.to_crs_string(); + + let mut out = savvy::OwnedListSexp::new(4, true)?; + out.set_name(0, "authority_code")?; + out.set_name(1, "srid")?; + out.set_name(2, "name")?; + out.set_name(3, "proj_string")?; + + if let Some(auth_code) = auth_code { + out.set_value(0, savvy::Sexp::try_from(auth_code.as_str())?)?; + } else { + out.set_value(0, savvy::NullSexp)?; + } + + if let Some(srid) = srid { + out.set_value(1, savvy::Sexp::try_from(srid as i32)?)?; + } else { + out.set_value(1, savvy::NullSexp)?; + } + + if let Some(name) = name { + out.set_value(2, savvy::Sexp::try_from(name)?)?; + } else { + out.set_value(2, savvy::NullSexp)?; + } + out.set_value(3, savvy::Sexp::try_from(proj_string.as_str())?)?; + + Ok(out.into()) } - out.set_value(3, savvy::Sexp::try_from(proj_string.as_str())?)?; - - Ok(out.into()) + None => Ok(savvy::NullSexp.into()), } - None => Ok(savvy::NullSexp.into()), + } else { + Ok(savvy::NullSexp.into()) } } diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index e00c8a13d..bdcac53a4 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -49,7 +49,7 @@ test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { expect_null(parsed$authority_code) expect_null(parsed$srid) expect_identical(parsed$name, "Construction Site Local Grid") - expect_true(!is.null(parsed$proj_string)) + expect_false(is.null(parsed$proj_string)) }) test_that("sd_parse_crs returns NULL if crs field is missing", { @@ -71,10 +71,10 @@ test_that("sd_parse_crs works with plain strings if that's what's in 'crs'", { # for consistent axis order in WKT/GeoJSON contexts. expect_identical(parsed$authority_code, "OGC:CRS84") expect_identical(parsed$srid, 4326L) - expect_true(!is.null(parsed$proj_string)) + expect_false(is.null(parsed$proj_string)) }) -# Tests for CRS display in print.sedonadb_dataframe (lines 325-360 of dataframe.R) +# Tests for CRS display in print.sedonadb_dataframe test_that("print.sedonadb_dataframe shows CRS info for geometry column with EPSG", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") @@ -137,9 +137,9 @@ test_that("print.sedonadb_dataframe respects width parameter for geometry line", output <- capture.output(print(df, n = 0, width = 60)) geo_line <- grep("^# Geometry:", output, value = TRUE) - if (length(geo_line) > 0) { - # Line should be truncated with "..." - expect_lte(nchar(geo_line), 60) - expect_match(geo_line, "\\.\\.\\.$") - } + expect_length(geo_line, 1) + + # Line should be truncated with "..." + expect_lte(nchar(geo_line), 60) + expect_match(geo_line, "\\.\\.\\.$") }) From 17cbc50d8b384a58afd07d58f67becda8dfc3888 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 11:06:36 +0100 Subject: [PATCH 08/18] add savvy update --- r/sedonadb/tools/savvy-update.sh | 92 ++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100755 r/sedonadb/tools/savvy-update.sh diff --git a/r/sedonadb/tools/savvy-update.sh b/r/sedonadb/tools/savvy-update.sh new file mode 100755 index 000000000..ce887eda9 --- /dev/null +++ b/r/sedonadb/tools/savvy-update.sh @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +set -eu + +main() { + local -r source_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + local -r source_rpkg_dir="$(cd "${source_dir}/../" && pwd)" + + # Run the updater + savvy-cli update "${source_rpkg_dir}" + + # Post-process files + local -r api_h="${source_rpkg_dir}/src/rust/api.h" + local -r init_c="${source_rpkg_dir}/src/init.c" + local -r wrappers_r="${source_rpkg_dir}/R/000-wrappers.R" + + mv "${api_h}" "${api_h}.tmp" + mv "${init_c}" "${init_c}.tmp" + mv "${wrappers_r}" "${wrappers_r}.tmp" + + # Add license header to api.h + echo "${LICENSE_C}" > "${api_h}" + cat "${api_h}.tmp" >> "${api_h}" + + # Add license header, put includes on their own lines, and fix a typo in init.c + echo "${LICENSE_C}" > "${init_c}" + cat "${init_c}.tmp" >> "${init_c}" + + # Add license header to 000-wrappers.R + echo "${LICENSE_R}" > "${wrappers_r}" + cat "${wrappers_r}.tmp" >> "${wrappers_r}" + + # Run clang-format on the generated C files + clang-format -i "${api_h}" + clang-format -i "${init_c}" + + # Remove .tmp files + rm "${api_h}.tmp" "${init_c}.tmp" "${wrappers_r}.tmp" +} + +LICENSE_R='# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +' +LICENSE_C='// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +' + +main From 26789b0eac331cad957b837d35ad677581260dcf Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 11:13:31 +0100 Subject: [PATCH 09/18] run savvy update --- r/sedonadb/R/000-wrappers.R | 22 +++++++--------------- r/sedonadb/src/init.c | 17 ++++++++++------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index 9740d1d67..140b1b49a 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -55,27 +55,18 @@ NULL } -`configure_proj_shared` <- function( - `shared_library_path` = NULL, - `database_path` = NULL, - `search_path` = NULL -) { - invisible(.Call( - savvy_configure_proj_shared__impl, - `shared_library_path`, - `database_path`, - `search_path` - )) +`configure_proj_shared` <- function(`shared_library_path` = NULL, `database_path` = NULL, `search_path` = NULL) { + invisible(.Call(savvy_configure_proj_shared__impl, `shared_library_path`, `database_path`, `search_path`)) } -`parse_crs_metadata` <- function(`crs_json`) { - .Call(savvy_parse_crs_metadata__impl, `crs_json`) +`init_r_runtime_interrupts` <- function(`interrupts_call`, `pkg_env`) { + invisible(.Call(savvy_init_r_runtime_interrupts__impl, `interrupts_call`, `pkg_env`)) } -`init_r_runtime_interrupts` <- function(`interrupts_call`, `pkg_env`) { - invisible(.Call(savvy_init_r_runtime_interrupts__impl, `interrupts_call`, `pkg_env`)) +`parse_crs_metadata` <- function(`crs_json`) { + .Call(savvy_parse_crs_metadata__impl, `crs_json`) } @@ -279,3 +270,4 @@ class(`InternalDataFrame`) <- c("sedonadb::InternalDataFrame__bundle", "savvy_se `print.sedonadb::InternalDataFrame__bundle` <- function(x, ...) { cat('sedonadb::InternalDataFrame\n') } + diff --git a/r/sedonadb/src/init.c b/r/sedonadb/src/init.c index 14af68ec7..9366d3120 100644 --- a/r/sedonadb/src/init.c +++ b/r/sedonadb/src/init.c @@ -15,11 +15,14 @@ // specific language governing permissions and limitations // under the License. -#include +// clang-format sorts includes unless SortIncludes: Never. However, the ordering +// does matter here. So, we need to disable clang-format for safety. +// clang-format off +#include #include - #include +// clang-format on #include "rust/api.h" @@ -60,11 +63,6 @@ SEXP savvy_configure_proj_shared__impl(SEXP c_arg__shared_library_path, return handle_result(res); } -SEXP savvy_parse_crs_metadata__impl(SEXP c_arg__crs_json) { - SEXP res = savvy_parse_crs_metadata__ffi(c_arg__crs_json); - return handle_result(res); -} - SEXP savvy_init_r_runtime__impl(DllInfo *c_arg___dll_info) { SEXP res = savvy_init_r_runtime__ffi(c_arg___dll_info); return handle_result(res); @@ -77,6 +75,11 @@ SEXP savvy_init_r_runtime_interrupts__impl(SEXP c_arg__interrupts_call, return handle_result(res); } +SEXP savvy_parse_crs_metadata__impl(SEXP c_arg__crs_json) { + SEXP res = savvy_parse_crs_metadata__ffi(c_arg__crs_json); + return handle_result(res); +} + SEXP savvy_sedonadb_adbc_init_func__impl(void) { SEXP res = savvy_sedonadb_adbc_init_func__ffi(); return handle_result(res); From 5bfc62bfbda5aee1ab34f155f66bb7cdfc9023a5 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 11:14:49 +0100 Subject: [PATCH 10/18] add snaps in tests for sd_parse_crs --- r/sedonadb/tests/testthat/_snaps/crs.md | 146 ++++++++++++++++++++++++ r/sedonadb/tests/testthat/test-crs.R | 68 ++--------- 2 files changed, 158 insertions(+), 56 deletions(-) create mode 100644 r/sedonadb/tests/testthat/_snaps/crs.md diff --git a/r/sedonadb/tests/testthat/_snaps/crs.md b/r/sedonadb/tests/testthat/_snaps/crs.md new file mode 100644 index 000000000..14149fa1f --- /dev/null +++ b/r/sedonadb/tests/testthat/_snaps/crs.md @@ -0,0 +1,146 @@ +# sd_parse_crs works for GeoArrow metadata with EPSG + + Code + sedonadb:::sd_parse_crs(meta) + Output + $authority_code + [1] "EPSG:5070" + + $srid + [1] 5070 + + $name + [1] "NAD83 / Conus Albers" + + $proj_string + [1] "{\"id\":{\"authority\":\"EPSG\",\"code\":5070},\"name\":\"NAD83 / Conus Albers\"}" + + +# sd_parse_crs works for Engineering CRS (no EPSG ID) + + Code + sedonadb:::sd_parse_crs(meta) + Output + $authority_code + NULL + + $srid + NULL + + $name + [1] "Construction Site Local Grid" + + $proj_string + [1] "{\"coordinate_system\":{\"axis\":[{\"abbreviation\":\"N\",\"direction\":\"north\",\"name\":\"Northing\",\"unit\":\"metre\"},{\"abbreviation\":\"E\",\"direction\":\"east\",\"name\":\"Easting\",\"unit\":\"metre\"}],\"subtype\":\"Cartesian\"},\"datum\":{\"name\":\"Local Datum\",\"type\":\"EngineeringDatum\"},\"name\":\"Construction Site Local Grid\",\"type\":\"EngineeringCRS\"}" + + +# sd_parse_crs returns NULL if crs field is missing + + Code + sedonadb:::sd_parse_crs("{\"something_else\": 123}") + Output + NULL + +--- + + Code + sedonadb:::sd_parse_crs("{}") + Output + NULL + +# sd_parse_crs handles invalid JSON gracefully + + Code + sedonadb:::sd_parse_crs("invalid json") + Condition + Error: + ! Failed to parse metadata JSON: expected value at line 1 column 1 + +# sd_parse_crs works with plain strings if that's what's in 'crs' + + Code + sedonadb:::sd_parse_crs(meta) + Output + $authority_code + [1] "OGC:CRS84" + + $srid + [1] 4326 + + $name + NULL + + $proj_string + [1] "OGC:CRS84" + + +# print.sedonadb_dataframe shows CRS info for geometry column with EPSG + + Code + print(df, n = 0) + Output + # A sedonadb_dataframe: ? x 1 + # Geometry: geom (CRS: OGC:CRS84) + +----------+ + | geom | + | geometry | + +----------+ + +----------+ + Preview of up to 0 row(s) + +# print.sedonadb_dataframe shows CRS info with different SRID + + Code + print(df, n = 0) + Output + # A sedonadb_dataframe: ? x 1 + # Geometry: geom (CRS: EPSG:5070) + +----------+ + | geom | + | geometry | + +----------+ + +----------+ + Preview of up to 0 row(s) + +# print.sedonadb_dataframe shows multiple geometry columns with CRS + + Code + print(df, n = 0) + Output + # A sedonadb_dataframe: ? x 2 + # Geometry: geom1 (CRS: OGC:CRS84), geom2 (CRS: EPSG:5070) + +----------+----------+ + | geom1 | geom2 | + | geometry | geometry | + +----------+----------+ + +----------+----------+ + Preview of up to 0 row(s) + +# print.sedonadb_dataframe handles geometry without explicit CRS + + Code + print(df, n = 0) + Output + # A sedonadb_dataframe: ? x 1 + # Geometry: geom (CRS: parsing error) + +----------+ + | geom | + | geometry | + +----------+ + +----------+ + Preview of up to 0 row(s) + +# print.sedonadb_dataframe respects width parameter for geometry line + + Code + print(df, n = 0, width = 60) + Output + # A sedonadb_dataframe: ? x 2 + # Geometry: very_long_geometry_column_name_1 (CRS: OGC:CR... + +-----------------------------+----------------------------+ + | very_long_geometry_column_n | very_long_geometry_column_ | + | ame_1... | name_2... | + +-----------------------------+----------------------------+ + +-----------------------------+----------------------------+ + Preview of up to 0 row(s) + diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index bdcac53a4..d7989c31d 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -17,13 +17,7 @@ test_that("sd_parse_crs works for GeoArrow metadata with EPSG", { meta <- '{"crs": {"id": {"authority": "EPSG", "code": 5070}, "name": "NAD83 / Conus Albers"}}' - parsed <- sedonadb:::sd_parse_crs(meta) - expect_identical(parsed$authority_code, "EPSG:5070") - expect_identical(parsed$srid, 5070L) - expect_identical(parsed$name, "NAD83 / Conus Albers") - # The proj_string is the *unwrapped* and *minified* PROJJSON content - expect_match(parsed$proj_string, '"authority":"EPSG"', fixed = TRUE) - expect_match(parsed$proj_string, '"code":5070', fixed = TRUE) + expect_snapshot(sedonadb:::sd_parse_crs(meta)) }) test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { @@ -45,57 +39,36 @@ test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { } } }' - parsed <- sedonadb:::sd_parse_crs(meta) - expect_null(parsed$authority_code) - expect_null(parsed$srid) - expect_identical(parsed$name, "Construction Site Local Grid") - expect_false(is.null(parsed$proj_string)) + expect_snapshot(sedonadb:::sd_parse_crs(meta)) }) test_that("sd_parse_crs returns NULL if crs field is missing", { - expect_null(sedonadb:::sd_parse_crs('{"something_else": 123}')) - expect_null(sedonadb:::sd_parse_crs('{}')) + expect_snapshot(sedonadb:::sd_parse_crs('{"something_else": 123}')) + expect_snapshot(sedonadb:::sd_parse_crs('{}')) }) test_that("sd_parse_crs handles invalid JSON gracefully", { - expect_error( + expect_snapshot( sedonadb:::sd_parse_crs('invalid json'), - "Failed to parse metadata JSON" + error = TRUE ) }) test_that("sd_parse_crs works with plain strings if that's what's in 'crs'", { meta <- '{"crs": "EPSG:4326"}' - parsed <- sedonadb:::sd_parse_crs(meta) - # Note: PROJ/sedona normalizes EPSG:4326 (lat/lon) to OGC:CRS84 (lon/lat) - # for consistent axis order in WKT/GeoJSON contexts. - expect_identical(parsed$authority_code, "OGC:CRS84") - expect_identical(parsed$srid, 4326L) - expect_false(is.null(parsed$proj_string)) + expect_snapshot(sedonadb:::sd_parse_crs(meta)) }) # Tests for CRS display in print.sedonadb_dataframe test_that("print.sedonadb_dataframe shows CRS info for geometry column with EPSG", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") - output <- capture.output(print(df, n = 0)) - - # Check that the Geometry line is present - - geo_line <- grep("^# Geometry:", output, value = TRUE) - expect_length(geo_line, 1) - - # Should show CRS information (OGC:CRS84 or EPSG:4326) - expect_match(geo_line, "geom .*(CRS: OGC:CRS84|CRS: EPSG:4326)") + expect_snapshot(print(df, n = 0)) }) test_that("print.sedonadb_dataframe shows CRS info with different SRID", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") - output <- capture.output(print(df, n = 0)) - - geo_line <- grep("^# Geometry:", output, value = TRUE) - expect_length(geo_line, 1) - expect_match(geo_line, "geom .*(CRS: EPSG:5070|CRS:.*5070)") + expect_snapshot(print(df, n = 0)) }) test_that("print.sedonadb_dataframe shows multiple geometry columns with CRS", { @@ -106,23 +79,13 @@ test_that("print.sedonadb_dataframe shows multiple geometry columns with CRS", { ST_SetSRID(ST_Point(3, 4), 5070) as geom2 " ) - output <- capture.output(print(df, n = 0)) - - geo_line <- grep("^# Geometry:", output, value = TRUE) - expect_length(geo_line, 1) - # Should contain both geometry columns - expect_match(geo_line, "geom1") - expect_match(geo_line, "geom2") + expect_snapshot(print(df, n = 0)) }) test_that("print.sedonadb_dataframe handles geometry without explicit CRS", { # ST_Point without ST_SetSRID may not have CRS metadata df <- sd_sql("SELECT ST_Point(1, 2) as geom") - output <- capture.output(print(df, n = 0)) - - # May or may not have a Geometry line depending on extension metadata - # At least it should not error - expect_true(any(grepl("sedonadb_dataframe", output))) + expect_snapshot(print(df, n = 0)) }) test_that("print.sedonadb_dataframe respects width parameter for geometry line", { @@ -134,12 +97,5 @@ test_that("print.sedonadb_dataframe respects width parameter for geometry line", " ) # Use a narrow width to trigger truncation - output <- capture.output(print(df, n = 0, width = 60)) - - geo_line <- grep("^# Geometry:", output, value = TRUE) - expect_length(geo_line, 1) - - # Line should be truncated with "..." - expect_lte(nchar(geo_line), 60) - expect_match(geo_line, "\\.\\.\\.$") + expect_snapshot(print(df, n = 0, width = 60)) }) From 24258b72435ec5c89204e9ca2acb6b552275d4a9 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 11:59:53 +0100 Subject: [PATCH 11/18] rust to parse crs instead of R --- r/sedonadb/R/000-wrappers.R | 52 ++++++++++++++++++++++++++++++ r/sedonadb/R/dataframe.R | 29 +++++------------ r/sedonadb/src/init.c | 26 +++++++++++++++ r/sedonadb/src/rust/api.h | 6 ++++ r/sedonadb/src/rust/src/ffi.rs | 6 ++++ r/sedonadb/src/rust/src/lib.rs | 58 ++++++++++++++++++++++++++++++++++ 6 files changed, 156 insertions(+), 21 deletions(-) diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index 140b1b49a..889e64fee 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -271,3 +271,55 @@ class(`InternalDataFrame`) <- c("sedonadb::InternalDataFrame__bundle", "savvy_se cat('sedonadb::InternalDataFrame\n') } +### wrapper functions for SedonaTypeR + +`SedonaTypeR_crs_display` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_crs_display__impl, `self`) + } +} + +`SedonaTypeR_logical_type_name` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_logical_type_name__impl, `self`) + } +} + +`SedonaTypeR_name` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_name__impl, `self`) + } +} + +`.savvy_wrap_SedonaTypeR` <- function(ptr) { + e <- new.env(parent = emptyenv()) + e$.ptr <- ptr + e$`crs_display` <- `SedonaTypeR_crs_display`(ptr) + e$`logical_type_name` <- `SedonaTypeR_logical_type_name`(ptr) + e$`name` <- `SedonaTypeR_name`(ptr) + + class(e) <- c("sedonadb::SedonaTypeR", "SedonaTypeR", "savvy_sedonadb__sealed") + e +} + + +#' R-exposed wrapper for SedonaType introspection +#' +#' This allows R code to inspect Arrow schema fields and determine +#' if they are geometry types with CRS information. +`SedonaTypeR` <- new.env(parent = emptyenv()) + +### associated functions for SedonaTypeR + +`SedonaTypeR`$`new` <- function(`schema_xptr`) { + .savvy_wrap_SedonaTypeR(.Call(savvy_SedonaTypeR_new__impl, `schema_xptr`)) +} + + +class(`SedonaTypeR`) <- c("sedonadb::SedonaTypeR__bundle", "savvy_sedonadb__sealed") + +#' @export +`print.sedonadb::SedonaTypeR__bundle` <- function(x, ...) { + cat('sedonadb::SedonaTypeR\n') +} + diff --git a/r/sedonadb/R/dataframe.R b/r/sedonadb/R/dataframe.R index 06d2847b4..f35d2f850 100644 --- a/r/sedonadb/R/dataframe.R +++ b/r/sedonadb/R/dataframe.R @@ -322,33 +322,20 @@ print.sedonadb_dataframe <- function(x, ..., width = NULL, n = NULL) { cat(sprintf("# A sedonadb_dataframe: ? x %d\n", ncols)) - # Print geometry column info - # we just use sd_parse_crs() to extract CRS info from ARROW:extension:metadata + # Print geometry column info using SedonaTypeR wrapper geo_col_info <- character() for (col_name in names(schema$children)) { child <- schema$children[[col_name]] - ext_name <- child$metadata[["ARROW:extension:name"]] - if (!is.null(ext_name) && grepl("^geoarrow\\.", ext_name)) { - ext_meta <- child$metadata[["ARROW:extension:metadata"]] - crs_info <- "" - if (!is.null(ext_meta)) { - parsed <- tryCatch( - sd_parse_crs(ext_meta), + sd_type <- tryCatch( + SedonaTypeR$new(child), error = function(e) NULL ) - if (is.null(parsed)) { - crs_info <- " (CRS: parsing error)" - } else if (!is.null(parsed$authority_code)) { - crs_info <- sprintf(" (CRS: %s)", parsed$authority_code) - } else if (!is.null(parsed$srid)) { - crs_info <- sprintf(" (CRS: EPSG:%d)", parsed$srid) - } else if (!is.null(parsed$name)) { - crs_info <- sprintf(" (CRS: %s)", parsed$name) - } else { - crs_info <- " (CRS: available)" - } + if (!is.null(sd_type)) { + logical_type <- sd_type$logical_type_name() + if (logical_type == "geometry" || logical_type == "geography") { + crs_display <- sd_type$crs_display() + geo_col_info <- c(geo_col_info, sprintf("%s%s", col_name, crs_display)) } - geo_col_info <- c(geo_col_info, sprintf("%s%s", col_name, crs_info)) } } diff --git a/r/sedonadb/src/init.c b/r/sedonadb/src/init.c index 9366d3120..673c8d8fd 100644 --- a/r/sedonadb/src/init.c +++ b/r/sedonadb/src/init.c @@ -217,6 +217,26 @@ SEXP savvy_InternalDataFrame_to_view__impl(SEXP self__, SEXP c_arg__ctx, return handle_result(res); } +SEXP savvy_SedonaTypeR_crs_display__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_crs_display__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_logical_type_name__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_logical_type_name__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_name__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_name__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_new__impl(SEXP c_arg__schema_xptr) { + SEXP res = savvy_SedonaTypeR_new__ffi(c_arg__schema_xptr); + return handle_result(res); +} + static const R_CallMethodDef CallEntries[] = { {"savvy_configure_proj_shared__impl", (DL_FUNC)&savvy_configure_proj_shared__impl, 3}, @@ -268,6 +288,12 @@ static const R_CallMethodDef CallEntries[] = { (DL_FUNC)&savvy_InternalDataFrame_to_provider__impl, 1}, {"savvy_InternalDataFrame_to_view__impl", (DL_FUNC)&savvy_InternalDataFrame_to_view__impl, 4}, + {"savvy_SedonaTypeR_crs_display__impl", + (DL_FUNC)&savvy_SedonaTypeR_crs_display__impl, 1}, + {"savvy_SedonaTypeR_logical_type_name__impl", + (DL_FUNC)&savvy_SedonaTypeR_logical_type_name__impl, 1}, + {"savvy_SedonaTypeR_name__impl", (DL_FUNC)&savvy_SedonaTypeR_name__impl, 1}, + {"savvy_SedonaTypeR_new__impl", (DL_FUNC)&savvy_SedonaTypeR_new__impl, 1}, {NULL, NULL, 0}}; void R_init_sedonadb(DllInfo *dll) { diff --git a/r/sedonadb/src/rust/api.h b/r/sedonadb/src/rust/api.h index 0619cf0a9..a2f200dda 100644 --- a/r/sedonadb/src/rust/api.h +++ b/r/sedonadb/src/rust/api.h @@ -61,3 +61,9 @@ SEXP savvy_InternalDataFrame_to_provider__ffi(SEXP self__); SEXP savvy_InternalDataFrame_to_view__ffi(SEXP self__, SEXP c_arg__ctx, SEXP c_arg__table_ref, SEXP c_arg__overwrite); + +// methods and associated functions for SedonaTypeR +SEXP savvy_SedonaTypeR_crs_display__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_logical_type_name__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_name__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_new__ffi(SEXP c_arg__schema_xptr); diff --git a/r/sedonadb/src/rust/src/ffi.rs b/r/sedonadb/src/rust/src/ffi.rs index 4275e2648..4c5d27fbb 100644 --- a/r/sedonadb/src/rust/src/ffi.rs +++ b/r/sedonadb/src/rust/src/ffi.rs @@ -58,6 +58,12 @@ pub fn import_scalar_udf(mut scalar_udf_xptr: savvy::Sexp) -> savvy::Result savvy::Result { + let ffi_schema: &FFI_ArrowSchema = import_xptr(&mut xptr, "nanoarrow_schema")?; + arrow_schema::Field::try_from(ffi_schema).map_err(|e| savvy_err!("{e}")) +} + fn import_xptr<'a, T>(xptr: &'a mut savvy::Sexp, cls: &str) -> savvy::Result<&'a mut T> { if !xptr.is_external_pointer() { return Err(savvy_err!( diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 840863fa2..f8f76eefb 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -123,3 +123,61 @@ fn parse_crs_metadata(crs_json: &str) -> savvy::Result { Ok(savvy::NullSexp.into()) } } + + +/// R-exposed wrapper for SedonaType introspection +/// +/// This allows R code to inspect Arrow schema fields and determine +/// if they are geometry types with CRS information. +#[savvy] +pub struct SedonaTypeR { + inner: sedona_schema::datatypes::SedonaType, + name: String, +} + +#[savvy] +impl SedonaTypeR { + /// Create a SedonaTypeR from a nanoarrow schema (external pointer) + /// + /// The schema should be a single field (column) schema, not a struct schema. + fn new(schema_xptr: savvy::Sexp) -> savvy::Result { + use sedona_schema::datatypes::SedonaType; + + let field = crate::ffi::import_arrow_field(schema_xptr)?; + let name = field.name().clone(); + + // Use existing SedonaType infrastructure to parse the field + let inner = SedonaType::from_storage_field(&field) + .map_err(|e| savvy::Error::new(format!("Failed to create SedonaType: {e}")))?; + + Ok(SedonaTypeR { inner, name }) + } + + /// Get the logical type name ("geometry", "geography", "utf8", etc.) + fn logical_type_name(&self) -> savvy::Result { + savvy::Sexp::try_from(self.inner.logical_type_name().as_str()) + } + + /// Get the column name + fn name(&self) -> savvy::Result { + savvy::Sexp::try_from(self.name.as_str()) + } + + /// Get a formatted CRS display string like " (CRS: EPSG:4326)" or empty string + fn crs_display(&self) -> savvy::Result { + use sedona_schema::datatypes::SedonaType; + + match &self.inner { + SedonaType::Wkb(_, crs) | SedonaType::WkbView(_, crs) => { + if let Some(crs_ref) = crs { + // Use the Display impl which gives us nice formatted output + let display = format!(" (CRS: {})", crs_ref); + savvy::Sexp::try_from(display.as_str()) + } else { + savvy::Sexp::try_from("") + } + } + _ => savvy::Sexp::try_from(""), + } + } +} From 9996def8b69547ab6f48d4fe3a723193feb198ac Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 12:05:50 +0100 Subject: [PATCH 12/18] additional tests for crs --- r/sedonadb/tests/testthat/_snaps/crs.md | 54 ++++++++- r/sedonadb/tests/testthat/test-crs.R | 145 ++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 5 deletions(-) diff --git a/r/sedonadb/tests/testthat/_snaps/crs.md b/r/sedonadb/tests/testthat/_snaps/crs.md index 14149fa1f..1beaede3d 100644 --- a/r/sedonadb/tests/testthat/_snaps/crs.md +++ b/r/sedonadb/tests/testthat/_snaps/crs.md @@ -80,7 +80,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 1 - # Geometry: geom (CRS: OGC:CRS84) + # Geometry: geom (CRS: ogc:crs84) +----------+ | geom | | geometry | @@ -94,7 +94,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 1 - # Geometry: geom (CRS: EPSG:5070) + # Geometry: geom (CRS: epsg:5070) +----------+ | geom | | geometry | @@ -108,7 +108,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 2 - # Geometry: geom1 (CRS: OGC:CRS84), geom2 (CRS: EPSG:5070) + # Geometry: geom1 (CRS: ogc:crs84), geom2 (CRS: epsg:5070) +----------+----------+ | geom1 | geom2 | | geometry | geometry | @@ -122,7 +122,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 1 - # Geometry: geom (CRS: parsing error) + # Geometry: geom +----------+ | geom | | geometry | @@ -136,7 +136,7 @@ print(df, n = 0, width = 60) Output # A sedonadb_dataframe: ? x 2 - # Geometry: very_long_geometry_column_name_1 (CRS: OGC:CR... + # Geometry: very_long_geometry_column_name_1 (CRS: ogc:cr... +-----------------------------+----------------------------+ | very_long_geometry_column_n | very_long_geometry_column_ | | ame_1... | name_2... | @@ -144,3 +144,47 @@ +-----------------------------+----------------------------+ Preview of up to 0 row(s) +# sd_parse_crs handles empty string + + Code + sedonadb:::sd_parse_crs("") + Condition + Error: + ! Failed to parse metadata JSON: EOF while parsing a value at line 1 column 0 + +# sd_parse_crs handles CRS with only name, no ID + + Code + sedonadb:::sd_parse_crs(meta) + Output + $authority_code + NULL + + $srid + NULL + + $name + [1] "Custom Geographic CRS" + + $proj_string + [1] "{\"name\":\"Custom Geographic CRS\",\"type\":\"GeographicCRS\"}" + + +# sd_parse_crs handles OGC:CRS84 + + Code + sedonadb:::sd_parse_crs(meta) + Output + $authority_code + [1] "OGC:CRS84" + + $srid + [1] 4326 + + $name + NULL + + $proj_string + [1] "OGC:CRS84" + + diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index d7989c31d..7ca0c2fb1 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -99,3 +99,148 @@ test_that("print.sedonadb_dataframe respects width parameter for geometry line", # Use a narrow width to trigger truncation expect_snapshot(print(df, n = 0, width = 60)) }) + +# Additional edge cases for sd_parse_crs + +test_that("sd_parse_crs handles NULL input", { + expect_error( + sedonadb:::sd_parse_crs(NULL), + "must be character" + ) +}) + +test_that("sd_parse_crs handles empty string", { + expect_snapshot( + sedonadb:::sd_parse_crs(""), + error = TRUE + ) +}) + +test_that("sd_parse_crs handles CRS with only name, no ID", { + meta <- '{ + "crs": { + "type": "GeographicCRS", + "name": "Custom Geographic CRS" + } + }' + expect_snapshot(sedonadb:::sd_parse_crs(meta)) +}) + +test_that("sd_parse_crs handles OGC:CRS84", { + # Common case in GeoParquet/GeoArrow + + meta <- '{"crs": "OGC:CRS84"}' + expect_snapshot(sedonadb:::sd_parse_crs(meta)) +}) + +# CRS preservation through data creation paths + +test_that("CRS is preserved when creating from data.frame with geometry", { + df <- as_sedonadb_dataframe( + data.frame( + geom = wk::as_wkb(wk::wkt("POINT (0 1)", crs = "EPSG:32620")) + ) + ) + + re_df <- sd_collect(df) + crs <- wk::wk_crs(re_df$geom) + expect_false(is.null(crs)) + # Check that the CRS contains EPSG:32620 info + expect_true( + grepl("32620", as.character(crs)) || + grepl("32620", jsonlite::toJSON(crs, auto_unbox = TRUE)) + ) +}) + +test_that("CRS is preserved through nanoarrow stream roundtrip", { + r_df <- data.frame( + geom = wk::as_wkb(wk::wkt("POINT (0 1)", crs = "EPSG:4326")) + ) + + stream <- nanoarrow::as_nanoarrow_array_stream(r_df) + df <- as_sedonadb_dataframe(stream, lazy = FALSE) + re_df <- sd_collect(df) + + crs <- wk::wk_crs(re_df$geom) + expect_false(is.null(crs)) +}) + +test_that("Different CRS values are preserved independently", { + # Create geometry with non-default CRS + df <- sd_sql( + " + SELECT + ST_SetSRID(ST_Point(1, 2), 4326) as geom_wgs84, + ST_SetSRID(ST_Point(3, 4), 32632) as geom_utm + " + ) + + re_df <- sd_collect(df) + + # Both geometries should have CRS metadata + crs1 <- wk::wk_crs(re_df$geom_wgs84) + crs2 <- wk::wk_crs(re_df$geom_utm) + + expect_false(is.null(crs1)) + expect_false(is.null(crs2)) +}) + +# Parquet roundtrip with CRS + +test_that("CRS is preserved through parquet write/read", { + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") + + tmp_parquet_file <- tempfile(fileext = ".parquet") + on.exit(unlink(tmp_parquet_file)) + + sd_write_parquet(df, tmp_parquet_file) + df_roundtrip <- sd_read_parquet(tmp_parquet_file) + re_df <- sd_collect(df_roundtrip) + + # Verify geometry has CRS + crs <- wk::wk_crs(re_df$geom) + expect_false(is.null(crs)) +}) + +test_that("Non-standard CRS is preserved through parquet roundtrip", { + # Use a less common SRID (NAD83 / Conus Albers) + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") + + tmp_parquet_file <- tempfile(fileext = ".parquet") + on.exit(unlink(tmp_parquet_file)) + + sd_write_parquet(df, tmp_parquet_file) + df_roundtrip <- sd_read_parquet(tmp_parquet_file) + re_df <- sd_collect(df_roundtrip) + + crs <- wk::wk_crs(re_df$geom) + expect_false(is.null(crs)) + # Check CRS info contains 5070 + crs_str <- jsonlite::toJSON(crs, auto_unbox = TRUE) + expect_true(grepl("5070", crs_str) || grepl("Albers", crs_str)) +}) + +# Multiple geometry columns with different CRS through operations + +test_that("Multiple geometry columns preserve their CRS after operations", { + df <- sd_sql( + " + SELECT + ST_SetSRID(ST_Point(1, 2), 4326) as point_a, + ST_SetSRID(ST_Point(3, 4), 5070) as point_b, + 'test' as name + " + ) + + # Collect and check both CRS are preserved + + re_df <- sd_collect(df) + + crs_a <- wk::wk_crs(re_df$point_a) + crs_b <- wk::wk_crs(re_df$point_b) + + expect_false(is.null(crs_a)) + expect_false(is.null(crs_b)) + # They should be different + expect_false(identical(crs_a, crs_b)) +}) From c51a335c3c88b5ff29eebb967c4f788ad5e962f8 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 12:20:18 +0100 Subject: [PATCH 13/18] savvy powered SedonaCrsR wrapper --- r/sedonadb/R/000-wrappers.R | 69 ++++++++++++++++++++++++++++++++++ r/sedonadb/src/init.c | 40 ++++++++++++++++++++ r/sedonadb/src/rust/api.h | 8 ++++ r/sedonadb/src/rust/src/lib.rs | 65 ++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+) diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index 889e64fee..5a9430e40 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -271,8 +271,76 @@ class(`InternalDataFrame`) <- c("sedonadb::InternalDataFrame__bundle", "savvy_se cat('sedonadb::InternalDataFrame\n') } +### wrapper functions for SedonaCrsR + +`SedonaCrsR_authority_code` <- function(self) { + function() { + .Call(savvy_SedonaCrsR_authority_code__impl, `self`) + } +} + +`SedonaCrsR_display` <- function(self) { + function() { + .Call(savvy_SedonaCrsR_display__impl, `self`) + } +} + +`SedonaCrsR_srid` <- function(self) { + function() { + .Call(savvy_SedonaCrsR_srid__impl, `self`) + } +} + +`SedonaCrsR_to_crs_string` <- function(self) { + function() { + .Call(savvy_SedonaCrsR_to_crs_string__impl, `self`) + } +} + +`SedonaCrsR_to_json` <- function(self) { + function() { + .Call(savvy_SedonaCrsR_to_json__impl, `self`) + } +} + +`.savvy_wrap_SedonaCrsR` <- function(ptr) { + e <- new.env(parent = emptyenv()) + e$.ptr <- ptr + e$`authority_code` <- `SedonaCrsR_authority_code`(ptr) + e$`display` <- `SedonaCrsR_display`(ptr) + e$`srid` <- `SedonaCrsR_srid`(ptr) + e$`to_crs_string` <- `SedonaCrsR_to_crs_string`(ptr) + e$`to_json` <- `SedonaCrsR_to_json`(ptr) + + class(e) <- c("sedonadb::SedonaCrsR", "SedonaCrsR", "savvy_sedonadb__sealed") + e +} + + +#' R-exposed wrapper for CRS (Coordinate Reference System) introspection +#' +#' This wraps an Arc and exposes its methods to R. +`SedonaCrsR` <- new.env(parent = emptyenv()) + +### associated functions for SedonaCrsR + + + +class(`SedonaCrsR`) <- c("sedonadb::SedonaCrsR__bundle", "savvy_sedonadb__sealed") + +#' @export +`print.sedonadb::SedonaCrsR__bundle` <- function(x, ...) { + cat('sedonadb::SedonaCrsR\n') +} + ### wrapper functions for SedonaTypeR +`SedonaTypeR_crs` <- function(self) { + function() { + .savvy_wrap_SedonaCrsR(.Call(savvy_SedonaTypeR_crs__impl, `self`)) + } +} + `SedonaTypeR_crs_display` <- function(self) { function() { .Call(savvy_SedonaTypeR_crs_display__impl, `self`) @@ -294,6 +362,7 @@ class(`InternalDataFrame`) <- c("sedonadb::InternalDataFrame__bundle", "savvy_se `.savvy_wrap_SedonaTypeR` <- function(ptr) { e <- new.env(parent = emptyenv()) e$.ptr <- ptr + e$`crs` <- `SedonaTypeR_crs`(ptr) e$`crs_display` <- `SedonaTypeR_crs_display`(ptr) e$`logical_type_name` <- `SedonaTypeR_logical_type_name`(ptr) e$`name` <- `SedonaTypeR_name`(ptr) diff --git a/r/sedonadb/src/init.c b/r/sedonadb/src/init.c index 673c8d8fd..eedf09a0f 100644 --- a/r/sedonadb/src/init.c +++ b/r/sedonadb/src/init.c @@ -217,6 +217,36 @@ SEXP savvy_InternalDataFrame_to_view__impl(SEXP self__, SEXP c_arg__ctx, return handle_result(res); } +SEXP savvy_SedonaCrsR_authority_code__impl(SEXP self__) { + SEXP res = savvy_SedonaCrsR_authority_code__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaCrsR_display__impl(SEXP self__) { + SEXP res = savvy_SedonaCrsR_display__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaCrsR_srid__impl(SEXP self__) { + SEXP res = savvy_SedonaCrsR_srid__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaCrsR_to_crs_string__impl(SEXP self__) { + SEXP res = savvy_SedonaCrsR_to_crs_string__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaCrsR_to_json__impl(SEXP self__) { + SEXP res = savvy_SedonaCrsR_to_json__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_crs__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_crs__ffi(self__); + return handle_result(res); +} + SEXP savvy_SedonaTypeR_crs_display__impl(SEXP self__) { SEXP res = savvy_SedonaTypeR_crs_display__ffi(self__); return handle_result(res); @@ -288,6 +318,16 @@ static const R_CallMethodDef CallEntries[] = { (DL_FUNC)&savvy_InternalDataFrame_to_provider__impl, 1}, {"savvy_InternalDataFrame_to_view__impl", (DL_FUNC)&savvy_InternalDataFrame_to_view__impl, 4}, + {"savvy_SedonaCrsR_authority_code__impl", + (DL_FUNC)&savvy_SedonaCrsR_authority_code__impl, 1}, + {"savvy_SedonaCrsR_display__impl", (DL_FUNC)&savvy_SedonaCrsR_display__impl, + 1}, + {"savvy_SedonaCrsR_srid__impl", (DL_FUNC)&savvy_SedonaCrsR_srid__impl, 1}, + {"savvy_SedonaCrsR_to_crs_string__impl", + (DL_FUNC)&savvy_SedonaCrsR_to_crs_string__impl, 1}, + {"savvy_SedonaCrsR_to_json__impl", (DL_FUNC)&savvy_SedonaCrsR_to_json__impl, + 1}, + {"savvy_SedonaTypeR_crs__impl", (DL_FUNC)&savvy_SedonaTypeR_crs__impl, 1}, {"savvy_SedonaTypeR_crs_display__impl", (DL_FUNC)&savvy_SedonaTypeR_crs_display__impl, 1}, {"savvy_SedonaTypeR_logical_type_name__impl", diff --git a/r/sedonadb/src/rust/api.h b/r/sedonadb/src/rust/api.h index a2f200dda..7e583f12c 100644 --- a/r/sedonadb/src/rust/api.h +++ b/r/sedonadb/src/rust/api.h @@ -62,7 +62,15 @@ SEXP savvy_InternalDataFrame_to_view__ffi(SEXP self__, SEXP c_arg__ctx, SEXP c_arg__table_ref, SEXP c_arg__overwrite); +// methods and associated functions for SedonaCrsR +SEXP savvy_SedonaCrsR_authority_code__ffi(SEXP self__); +SEXP savvy_SedonaCrsR_display__ffi(SEXP self__); +SEXP savvy_SedonaCrsR_srid__ffi(SEXP self__); +SEXP savvy_SedonaCrsR_to_crs_string__ffi(SEXP self__); +SEXP savvy_SedonaCrsR_to_json__ffi(SEXP self__); + // methods and associated functions for SedonaTypeR +SEXP savvy_SedonaTypeR_crs__ffi(SEXP self__); SEXP savvy_SedonaTypeR_crs_display__ffi(SEXP self__); SEXP savvy_SedonaTypeR_logical_type_name__ffi(SEXP self__); SEXP savvy_SedonaTypeR_name__ffi(SEXP self__); diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index f8f76eefb..8d4b41f7b 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -17,12 +17,14 @@ // Example functions use std::ffi::c_void; +use std::sync::Arc; use savvy::savvy; use savvy_ffi::R_NilValue; use sedona_adbc::AdbcSedonadbDriverInit; use sedona_proj::register::{configure_global_proj_engine, ProjCrsEngineBuilder}; +use sedona_schema::crs::CoordinateReferenceSystem; mod context; mod dataframe; @@ -124,6 +126,51 @@ fn parse_crs_metadata(crs_json: &str) -> savvy::Result { } } +/// R-exposed wrapper for CRS (Coordinate Reference System) introspection +/// +/// This wraps an Arc and exposes its methods to R. +#[savvy] +pub struct SedonaCrsR { + inner: Arc, +} + +#[savvy] +impl SedonaCrsR { + /// Get the SRID (e.g., 4326 for WGS84) or NULL if not an EPSG code + fn srid(&self) -> savvy::Result { + match self.inner.srid() { + Ok(Some(srid)) => savvy::Sexp::try_from(srid as i32), + Ok(None) => Ok(savvy::NullSexp.into()), + Err(e) => Err(savvy::Error::new(format!("Failed to get SRID: {e}"))), + } + } + + /// Get the authority code (e.g., "EPSG:4326") or NULL if not available + fn authority_code(&self) -> savvy::Result { + match self.inner.to_authority_code() { + Ok(Some(code)) => savvy::Sexp::try_from(code.as_str()), + Ok(None) => Ok(savvy::NullSexp.into()), + Err(e) => Err(savvy::Error::new(format!("Failed to get authority code: {e}"))), + } + } + + /// Get the JSON representation of the CRS + fn to_json(&self) -> savvy::Result { + savvy::Sexp::try_from(self.inner.to_json().as_str()) + } + + /// Get the PROJ-compatible CRS string representation + fn to_crs_string(&self) -> savvy::Result { + savvy::Sexp::try_from(self.inner.to_crs_string().as_str()) + } + + /// Get a formatted display string (e.g., "epsg:4326" or "{...}") + fn display(&self) -> savvy::Result { + let display = format!("{}", self.inner.as_ref()); + savvy::Sexp::try_from(display.as_str()) + } +} + /// R-exposed wrapper for SedonaType introspection /// @@ -163,6 +210,24 @@ impl SedonaTypeR { savvy::Sexp::try_from(self.name.as_str()) } + /// Get the CRS wrapper object, or NULL if no CRS is present + /// + /// This returns a SedonaCrsR object that can be used to inspect the CRS. + fn crs(&self) -> savvy::Result { + use sedona_schema::datatypes::SedonaType; + + match &self.inner { + SedonaType::Wkb(_, crs) | SedonaType::WkbView(_, crs) => { + if let Some(crs_arc) = crs { + Ok(SedonaCrsR { inner: crs_arc.clone() }) + } else { + Err(savvy::Error::new("No CRS available for this geometry type")) + } + } + _ => Err(savvy::Error::new("No CRS available for non-geometry types")), + } + } + /// Get a formatted CRS display string like " (CRS: EPSG:4326)" or empty string fn crs_display(&self) -> savvy::Result { use sedona_schema::datatypes::SedonaType; From 712d2b27f6767609d9f12117d4dbd41a1ef03cc7 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 12:27:53 +0100 Subject: [PATCH 14/18] correct crs display --- r/sedonadb/src/rust/src/lib.rs | 18 +++++++++++---- r/sedonadb/tests/testthat/_snaps/crs.md | 29 +++++++++++++++++++++---- r/sedonadb/tests/testthat/test-crs.R | 20 +++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 8d4b41f7b..2b06d0d6f 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -164,9 +164,13 @@ impl SedonaCrsR { savvy::Sexp::try_from(self.inner.to_crs_string().as_str()) } - /// Get a formatted display string (e.g., "epsg:4326" or "{...}") + /// Get a formatted display string (e.g., "EPSG:4326" or "{...}") fn display(&self) -> savvy::Result { - let display = format!("{}", self.inner.as_ref()); + let display = if let Ok(Some(auth)) = self.inner.to_authority_code() { + auth + } else { + format!("{}", self.inner.as_ref()) + }; savvy::Sexp::try_from(display.as_str()) } } @@ -235,8 +239,14 @@ impl SedonaTypeR { match &self.inner { SedonaType::Wkb(_, crs) | SedonaType::WkbView(_, crs) => { if let Some(crs_ref) = crs { - // Use the Display impl which gives us nice formatted output - let display = format!(" (CRS: {})", crs_ref); + // Try to get authority code first (usually EPSG:XXXX) + let auth = crs_ref.to_authority_code().ok().flatten(); + let display = if let Some(auth) = auth { + format!(" (CRS: {})", auth) + } else { + // Fallback to the Display impl which might be lowercase or PROJJSON + format!(" (CRS: {})", crs_ref) + }; savvy::Sexp::try_from(display.as_str()) } else { savvy::Sexp::try_from("") diff --git a/r/sedonadb/tests/testthat/_snaps/crs.md b/r/sedonadb/tests/testthat/_snaps/crs.md index 1beaede3d..e576d60ba 100644 --- a/r/sedonadb/tests/testthat/_snaps/crs.md +++ b/r/sedonadb/tests/testthat/_snaps/crs.md @@ -80,7 +80,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 1 - # Geometry: geom (CRS: ogc:crs84) + # Geometry: geom (CRS: OGC:CRS84) +----------+ | geom | | geometry | @@ -94,7 +94,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 1 - # Geometry: geom (CRS: epsg:5070) + # Geometry: geom (CRS: EPSG:5070) +----------+ | geom | | geometry | @@ -108,7 +108,7 @@ print(df, n = 0) Output # A sedonadb_dataframe: ? x 2 - # Geometry: geom1 (CRS: ogc:crs84), geom2 (CRS: epsg:5070) + # Geometry: geom1 (CRS: OGC:CRS84), geom2 (CRS: EPSG:5070) +----------+----------+ | geom1 | geom2 | | geometry | geometry | @@ -136,7 +136,7 @@ print(df, n = 0, width = 60) Output # A sedonadb_dataframe: ? x 2 - # Geometry: very_long_geometry_column_name_1 (CRS: ogc:cr... + # Geometry: very_long_geometry_column_name_1 (CRS: OGC:CR... +-----------------------------+----------------------------+ | very_long_geometry_column_n | very_long_geometry_column_ | | ame_1... | name_2... | @@ -188,3 +188,24 @@ [1] "OGC:CRS84" +# SedonaTypeR$crs_display() uses uppercase authority codes + + Code + sd_type$crs_display() + Output + [1] " (CRS: OGC:CRS84)" + +--- + + Code + sd_type5070$crs_display() + Output + [1] " (CRS: EPSG:5070)" + +# SedonaCrsR$display() uses uppercase authority codes + + Code + crs$display() + Output + [1] "OGC:CRS84" + diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index 7ca0c2fb1..193c6d1ce 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -133,6 +133,26 @@ test_that("sd_parse_crs handles OGC:CRS84", { expect_snapshot(sedonadb:::sd_parse_crs(meta)) }) +# Explicit tests for Rust wrappers to ensure uppercase casing + +test_that("SedonaTypeR$crs_display() uses uppercase authority codes", { + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") + schema <- nanoarrow::infer_nanoarrow_schema(df) + sd_type <- sedonadb:::SedonaTypeR$new(schema$children$geom) + expect_snapshot(sd_type$crs_display()) + + df5070 <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") + sd_type5070 <- sedonadb:::SedonaTypeR$new(nanoarrow::infer_nanoarrow_schema(df5070)$children$geom) + expect_snapshot(sd_type5070$crs_display()) +}) + +test_that("SedonaCrsR$display() uses uppercase authority codes", { + df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") + sd_type <- sedonadb:::SedonaTypeR$new(nanoarrow::infer_nanoarrow_schema(df)$children$geom) + crs <- sd_type$crs() + expect_snapshot(crs$display()) +}) + # CRS preservation through data creation paths test_that("CRS is preserved when creating from data.frame with geometry", { From 905a1f939aaa573661c1b20075ff248814bc85d7 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 13:10:38 +0100 Subject: [PATCH 15/18] fix CI errors --- Cargo.lock | 1 + dev/release/rat_exclude_files.txt | 1 + r/sedonadb/R/000-wrappers.R | 1 - r/sedonadb/src/rust/src/ffi.rs | 1 - r/sedonadb/src/rust/src/lib.rs | 9 ++++++--- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a79e1c528..f57938281 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5333,6 +5333,7 @@ dependencies = [ "sedona-geoparquet", "sedona-proj", "sedona-schema", + "serde_json", "thiserror 2.0.17", "tokio", ] diff --git a/dev/release/rat_exclude_files.txt b/dev/release/rat_exclude_files.txt index 6938c0fc7..bad4cb5c6 100644 --- a/dev/release/rat_exclude_files.txt +++ b/dev/release/rat_exclude_files.txt @@ -25,4 +25,5 @@ r/sedonadb/.Rbuildignore r/sedonadb/DESCRIPTION r/sedonadb/NAMESPACE r/sedonadb/src/sedonadb-win.def +r/sedonadb/tests/testthat/_snaps/* submodules/geoarrow-data/* diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index 5a9430e40..8e078eb4f 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -391,4 +391,3 @@ class(`SedonaTypeR`) <- c("sedonadb::SedonaTypeR__bundle", "savvy_sedonadb__seal `print.sedonadb::SedonaTypeR__bundle` <- function(x, ...) { cat('sedonadb::SedonaTypeR\n') } - diff --git a/r/sedonadb/src/rust/src/ffi.rs b/r/sedonadb/src/rust/src/ffi.rs index 4c5d27fbb..bbe0af329 100644 --- a/r/sedonadb/src/rust/src/ffi.rs +++ b/r/sedonadb/src/rust/src/ffi.rs @@ -58,7 +58,6 @@ pub fn import_scalar_udf(mut scalar_udf_xptr: savvy::Sexp) -> savvy::Result savvy::Result { let ffi_schema: &FFI_ArrowSchema = import_xptr(&mut xptr, "nanoarrow_schema")?; arrow_schema::Field::try_from(ffi_schema).map_err(|e| savvy_err!("{e}")) diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 2b06d0d6f..98fe37177 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -150,7 +150,9 @@ impl SedonaCrsR { match self.inner.to_authority_code() { Ok(Some(code)) => savvy::Sexp::try_from(code.as_str()), Ok(None) => Ok(savvy::NullSexp.into()), - Err(e) => Err(savvy::Error::new(format!("Failed to get authority code: {e}"))), + Err(e) => Err(savvy::Error::new(format!( + "Failed to get authority code: {e}" + ))), } } @@ -175,7 +177,6 @@ impl SedonaCrsR { } } - /// R-exposed wrapper for SedonaType introspection /// /// This allows R code to inspect Arrow schema fields and determine @@ -223,7 +224,9 @@ impl SedonaTypeR { match &self.inner { SedonaType::Wkb(_, crs) | SedonaType::WkbView(_, crs) => { if let Some(crs_arc) = crs { - Ok(SedonaCrsR { inner: crs_arc.clone() }) + Ok(SedonaCrsR { + inner: crs_arc.clone(), + }) } else { Err(savvy::Error::new("No CRS available for this geometry type")) } From 21507be3d10252e9df572e8ace9524e7bdb7e983 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 7 Jan 2026 08:52:45 -0600 Subject: [PATCH 16/18] regenerate and format --- r/sedonadb/R/000-wrappers.R | 146 ++++++++++++++------------- r/sedonadb/R/dataframe.R | 4 +- r/sedonadb/src/init.c | 61 +++++------ r/sedonadb/src/rust/api.h | 13 +-- r/sedonadb/tests/testthat/test-crs.R | 8 +- 5 files changed, 124 insertions(+), 108 deletions(-) diff --git a/r/sedonadb/R/000-wrappers.R b/r/sedonadb/R/000-wrappers.R index 72b5d5549..10b904b8c 100644 --- a/r/sedonadb/R/000-wrappers.R +++ b/r/sedonadb/R/000-wrappers.R @@ -398,6 +398,24 @@ class(`InternalDataFrame`) <- c( e$`to_json` <- `SedonaCrsR_to_json`(ptr) class(e) <- c("sedonadb::SedonaCrsR", "SedonaCrsR", "savvy_sedonadb__sealed") + e +} + + +#' R-exposed wrapper for CRS (Coordinate Reference System) introspection +#' +#' This wraps an Arc and exposes its methods to R. +`SedonaCrsR` <- new.env(parent = emptyenv()) + +### associated functions for SedonaCrsR + +class(`SedonaCrsR`) <- c("sedonadb::SedonaCrsR__bundle", "savvy_sedonadb__sealed") + +#' @export +`print.sedonadb::SedonaCrsR__bundle` <- function(x, ...) { + cat('sedonadb::SedonaCrsR\n') +} + ### wrapper functions for SedonaDBExpr `SedonaDBExpr_alias` <- function(self) { @@ -444,57 +462,6 @@ class(`InternalDataFrame`) <- c( } -#' R-exposed wrapper for CRS (Coordinate Reference System) introspection -#' -#' This wraps an Arc and exposes its methods to R. -`SedonaCrsR` <- new.env(parent = emptyenv()) - -### associated functions for SedonaCrsR - - - -class(`SedonaCrsR`) <- c("sedonadb::SedonaCrsR__bundle", "savvy_sedonadb__sealed") - -#' @export -`print.sedonadb::SedonaCrsR__bundle` <- function(x, ...) { - cat('sedonadb::SedonaCrsR\n') -} - -### wrapper functions for SedonaTypeR - -`SedonaTypeR_crs` <- function(self) { - function() { - .savvy_wrap_SedonaCrsR(.Call(savvy_SedonaTypeR_crs__impl, `self`)) - } -} - -`SedonaTypeR_crs_display` <- function(self) { - function() { - .Call(savvy_SedonaTypeR_crs_display__impl, `self`) - } -} - -`SedonaTypeR_logical_type_name` <- function(self) { - function() { - .Call(savvy_SedonaTypeR_logical_type_name__impl, `self`) - } -} - -`SedonaTypeR_name` <- function(self) { - function() { - .Call(savvy_SedonaTypeR_name__impl, `self`) - } -} - -`.savvy_wrap_SedonaTypeR` <- function(ptr) { - e <- new.env(parent = emptyenv()) - e$.ptr <- ptr - e$`crs` <- `SedonaTypeR_crs`(ptr) - e$`crs_display` <- `SedonaTypeR_crs_display`(ptr) - e$`logical_type_name` <- `SedonaTypeR_logical_type_name`(ptr) - e$`name` <- `SedonaTypeR_name`(ptr) - - class(e) <- c("sedonadb::SedonaTypeR", "SedonaTypeR", "savvy_sedonadb__sealed") `SedonaDBExpr` <- new.env(parent = emptyenv()) ### associated functions for SedonaDBExpr @@ -574,24 +541,6 @@ class(`SedonaDBExpr`) <- c("sedonadb::SedonaDBExpr__bundle", "savvy_sedonadb__se } -#' R-exposed wrapper for SedonaType introspection -#' -#' This allows R code to inspect Arrow schema fields and determine -#' if they are geometry types with CRS information. -`SedonaTypeR` <- new.env(parent = emptyenv()) - -### associated functions for SedonaTypeR - -`SedonaTypeR`$`new` <- function(`schema_xptr`) { - .savvy_wrap_SedonaTypeR(.Call(savvy_SedonaTypeR_new__impl, `schema_xptr`)) -} - - -class(`SedonaTypeR`) <- c("sedonadb::SedonaTypeR__bundle", "savvy_sedonadb__sealed") - -#' @export -`print.sedonadb::SedonaTypeR__bundle` <- function(x, ...) { - cat('sedonadb::SedonaTypeR\n') `SedonaDBExprFactory` <- new.env(parent = emptyenv()) ### associated functions for SedonaDBExprFactory @@ -619,3 +568,62 @@ class(`SedonaDBExprFactory`) <- c( `print.sedonadb::SedonaDBExprFactory__bundle` <- function(x, ...) { cat('sedonadb::SedonaDBExprFactory\n') } + +### wrapper functions for SedonaTypeR + +`SedonaTypeR_crs` <- function(self) { + function() { + .savvy_wrap_SedonaCrsR(.Call(savvy_SedonaTypeR_crs__impl, `self`)) + } +} + +`SedonaTypeR_crs_display` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_crs_display__impl, `self`) + } +} + +`SedonaTypeR_logical_type_name` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_logical_type_name__impl, `self`) + } +} + +`SedonaTypeR_name` <- function(self) { + function() { + .Call(savvy_SedonaTypeR_name__impl, `self`) + } +} + +`.savvy_wrap_SedonaTypeR` <- function(ptr) { + e <- new.env(parent = emptyenv()) + e$.ptr <- ptr + e$`crs` <- `SedonaTypeR_crs`(ptr) + e$`crs_display` <- `SedonaTypeR_crs_display`(ptr) + e$`logical_type_name` <- `SedonaTypeR_logical_type_name`(ptr) + e$`name` <- `SedonaTypeR_name`(ptr) + + class(e) <- c("sedonadb::SedonaTypeR", "SedonaTypeR", "savvy_sedonadb__sealed") + e +} + + +#' R-exposed wrapper for SedonaType introspection +#' +#' This allows R code to inspect Arrow schema fields and determine +#' if they are geometry types with CRS information. +`SedonaTypeR` <- new.env(parent = emptyenv()) + +### associated functions for SedonaTypeR + +`SedonaTypeR`$`new` <- function(`schema_xptr`) { + .savvy_wrap_SedonaTypeR(.Call(savvy_SedonaTypeR_new__impl, `schema_xptr`)) +} + + +class(`SedonaTypeR`) <- c("sedonadb::SedonaTypeR__bundle", "savvy_sedonadb__sealed") + +#' @export +`print.sedonadb::SedonaTypeR__bundle` <- function(x, ...) { + cat('sedonadb::SedonaTypeR\n') +} diff --git a/r/sedonadb/R/dataframe.R b/r/sedonadb/R/dataframe.R index 56e3530e8..a7abffa81 100644 --- a/r/sedonadb/R/dataframe.R +++ b/r/sedonadb/R/dataframe.R @@ -333,8 +333,8 @@ print.sedonadb_dataframe <- function(x, ..., width = NULL, n = NULL) { child <- schema$children[[col_name]] sd_type <- tryCatch( SedonaTypeR$new(child), - error = function(e) NULL - ) + error = function(e) NULL + ) if (!is.null(sd_type)) { logical_type <- sd_type$logical_type_name() if (logical_type == "geometry" || logical_type == "geography") { diff --git a/r/sedonadb/src/init.c b/r/sedonadb/src/init.c index 9013c7984..40a49d4c8 100644 --- a/r/sedonadb/src/init.c +++ b/r/sedonadb/src/init.c @@ -242,28 +242,6 @@ SEXP savvy_SedonaCrsR_to_json__impl(SEXP self__) { return handle_result(res); } -SEXP savvy_SedonaTypeR_crs__impl(SEXP self__) { - SEXP res = savvy_SedonaTypeR_crs__ffi(self__); - return handle_result(res); -} - -SEXP savvy_SedonaTypeR_crs_display__impl(SEXP self__) { - SEXP res = savvy_SedonaTypeR_crs_display__ffi(self__); - return handle_result(res); -} - -SEXP savvy_SedonaTypeR_logical_type_name__impl(SEXP self__) { - SEXP res = savvy_SedonaTypeR_logical_type_name__ffi(self__); - return handle_result(res); -} - -SEXP savvy_SedonaTypeR_name__impl(SEXP self__) { - SEXP res = savvy_SedonaTypeR_name__ffi(self__); - return handle_result(res); -} - -SEXP savvy_SedonaTypeR_new__impl(SEXP c_arg__schema_xptr) { - SEXP res = savvy_SedonaTypeR_new__ffi(c_arg__schema_xptr); SEXP savvy_SedonaDBExpr_alias__impl(SEXP self__, SEXP c_arg__name) { SEXP res = savvy_SedonaDBExpr_alias__ffi(self__, c_arg__name); return handle_result(res); @@ -333,6 +311,31 @@ SEXP savvy_SedonaDBExprFactory_scalar_function__impl(SEXP self__, return handle_result(res); } +SEXP savvy_SedonaTypeR_crs__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_crs__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_crs_display__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_crs_display__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_logical_type_name__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_logical_type_name__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_name__impl(SEXP self__) { + SEXP res = savvy_SedonaTypeR_name__ffi(self__); + return handle_result(res); +} + +SEXP savvy_SedonaTypeR_new__impl(SEXP c_arg__schema_xptr) { + SEXP res = savvy_SedonaTypeR_new__ffi(c_arg__schema_xptr); + return handle_result(res); +} + static const R_CallMethodDef CallEntries[] = { {"savvy_configure_proj_shared__impl", (DL_FUNC)&savvy_configure_proj_shared__impl, 3}, @@ -393,13 +396,6 @@ static const R_CallMethodDef CallEntries[] = { (DL_FUNC)&savvy_SedonaCrsR_to_crs_string__impl, 1}, {"savvy_SedonaCrsR_to_json__impl", (DL_FUNC)&savvy_SedonaCrsR_to_json__impl, 1}, - {"savvy_SedonaTypeR_crs__impl", (DL_FUNC)&savvy_SedonaTypeR_crs__impl, 1}, - {"savvy_SedonaTypeR_crs_display__impl", - (DL_FUNC)&savvy_SedonaTypeR_crs_display__impl, 1}, - {"savvy_SedonaTypeR_logical_type_name__impl", - (DL_FUNC)&savvy_SedonaTypeR_logical_type_name__impl, 1}, - {"savvy_SedonaTypeR_name__impl", (DL_FUNC)&savvy_SedonaTypeR_name__impl, 1}, - {"savvy_SedonaTypeR_new__impl", (DL_FUNC)&savvy_SedonaTypeR_new__impl, 1}, {"savvy_SedonaDBExpr_alias__impl", (DL_FUNC)&savvy_SedonaDBExpr_alias__impl, 2}, {"savvy_SedonaDBExpr_cast__impl", (DL_FUNC)&savvy_SedonaDBExpr_cast__impl, @@ -422,6 +418,13 @@ static const R_CallMethodDef CallEntries[] = { (DL_FUNC)&savvy_SedonaDBExprFactory_new__impl, 1}, {"savvy_SedonaDBExprFactory_scalar_function__impl", (DL_FUNC)&savvy_SedonaDBExprFactory_scalar_function__impl, 3}, + {"savvy_SedonaTypeR_crs__impl", (DL_FUNC)&savvy_SedonaTypeR_crs__impl, 1}, + {"savvy_SedonaTypeR_crs_display__impl", + (DL_FUNC)&savvy_SedonaTypeR_crs_display__impl, 1}, + {"savvy_SedonaTypeR_logical_type_name__impl", + (DL_FUNC)&savvy_SedonaTypeR_logical_type_name__impl, 1}, + {"savvy_SedonaTypeR_name__impl", (DL_FUNC)&savvy_SedonaTypeR_name__impl, 1}, + {"savvy_SedonaTypeR_new__impl", (DL_FUNC)&savvy_SedonaTypeR_new__impl, 1}, {NULL, NULL, 0}}; void R_init_sedonadb(DllInfo *dll) { diff --git a/r/sedonadb/src/rust/api.h b/r/sedonadb/src/rust/api.h index 49683524c..f4e648c38 100644 --- a/r/sedonadb/src/rust/api.h +++ b/r/sedonadb/src/rust/api.h @@ -69,12 +69,6 @@ SEXP savvy_SedonaCrsR_srid__ffi(SEXP self__); SEXP savvy_SedonaCrsR_to_crs_string__ffi(SEXP self__); SEXP savvy_SedonaCrsR_to_json__ffi(SEXP self__); -// methods and associated functions for SedonaTypeR -SEXP savvy_SedonaTypeR_crs__ffi(SEXP self__); -SEXP savvy_SedonaTypeR_crs_display__ffi(SEXP self__); -SEXP savvy_SedonaTypeR_logical_type_name__ffi(SEXP self__); -SEXP savvy_SedonaTypeR_name__ffi(SEXP self__); -SEXP savvy_SedonaTypeR_new__ffi(SEXP c_arg__schema_xptr); // methods and associated functions for SedonaDBExpr SEXP savvy_SedonaDBExpr_alias__ffi(SEXP self__, SEXP c_arg__name); SEXP savvy_SedonaDBExpr_cast__ffi(SEXP self__, SEXP c_arg__schema_xptr); @@ -98,3 +92,10 @@ SEXP savvy_SedonaDBExprFactory_new__ffi(SEXP c_arg__ctx); SEXP savvy_SedonaDBExprFactory_scalar_function__ffi(SEXP self__, SEXP c_arg__name, SEXP c_arg__args); + +// methods and associated functions for SedonaTypeR +SEXP savvy_SedonaTypeR_crs__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_crs_display__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_logical_type_name__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_name__ffi(SEXP self__); +SEXP savvy_SedonaTypeR_new__ffi(SEXP c_arg__schema_xptr); diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index 193c6d1ce..2af8c264f 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -142,13 +142,17 @@ test_that("SedonaTypeR$crs_display() uses uppercase authority codes", { expect_snapshot(sd_type$crs_display()) df5070 <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") - sd_type5070 <- sedonadb:::SedonaTypeR$new(nanoarrow::infer_nanoarrow_schema(df5070)$children$geom) + sd_type5070 <- sedonadb:::SedonaTypeR$new( + nanoarrow::infer_nanoarrow_schema(df5070)$children$geom + ) expect_snapshot(sd_type5070$crs_display()) }) test_that("SedonaCrsR$display() uses uppercase authority codes", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") - sd_type <- sedonadb:::SedonaTypeR$new(nanoarrow::infer_nanoarrow_schema(df)$children$geom) + sd_type <- sedonadb:::SedonaTypeR$new( + nanoarrow::infer_nanoarrow_schema(df)$children$geom + ) crs <- sd_type$crs() expect_snapshot(crs$display()) }) From a1de7068a6129eb5c088f41b2029d1515d5b0395 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 18:10:13 +0100 Subject: [PATCH 17/18] remove redundant import_arrow_field --- r/sedonadb/src/rust/src/ffi.rs | 5 ----- r/sedonadb/src/rust/src/lib.rs | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/r/sedonadb/src/rust/src/ffi.rs b/r/sedonadb/src/rust/src/ffi.rs index cffdf0693..828364c38 100644 --- a/r/sedonadb/src/rust/src/ffi.rs +++ b/r/sedonadb/src/rust/src/ffi.rs @@ -78,11 +78,6 @@ pub fn import_scalar_udf(mut scalar_udf_xptr: savvy::Sexp) -> savvy::Result savvy::Result { - let ffi_schema: &FFI_ArrowSchema = import_xptr(&mut xptr, "nanoarrow_schema")?; - arrow_schema::Field::try_from(ffi_schema).map_err(|e| savvy_err!("{e}")) -} - fn import_xptr<'a, T>(xptr: &'a mut savvy::Sexp, cls: &str) -> savvy::Result<&'a mut T> { if !xptr.is_external_pointer() { return Err(savvy_err!( diff --git a/r/sedonadb/src/rust/src/lib.rs b/r/sedonadb/src/rust/src/lib.rs index 077604ff9..44747539e 100644 --- a/r/sedonadb/src/rust/src/lib.rs +++ b/r/sedonadb/src/rust/src/lib.rs @@ -196,7 +196,7 @@ impl SedonaTypeR { fn new(schema_xptr: savvy::Sexp) -> savvy::Result { use sedona_schema::datatypes::SedonaType; - let field = crate::ffi::import_arrow_field(schema_xptr)?; + let field = crate::ffi::import_field(schema_xptr)?; let name = field.name().clone(); // Use existing SedonaType infrastructure to parse the field From 53a9d102d5565f2b5b66f513f8b2b3a923adc074 Mon Sep 17 00:00:00 2001 From: Egor Kotov Date: Wed, 7 Jan 2026 18:14:12 +0100 Subject: [PATCH 18/18] remove redunant ::: in tests --- r/sedonadb/tests/testthat/test-crs.R | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/r/sedonadb/tests/testthat/test-crs.R b/r/sedonadb/tests/testthat/test-crs.R index 2af8c264f..8fc88a4ae 100644 --- a/r/sedonadb/tests/testthat/test-crs.R +++ b/r/sedonadb/tests/testthat/test-crs.R @@ -17,7 +17,7 @@ test_that("sd_parse_crs works for GeoArrow metadata with EPSG", { meta <- '{"crs": {"id": {"authority": "EPSG", "code": 5070}, "name": "NAD83 / Conus Albers"}}' - expect_snapshot(sedonadb:::sd_parse_crs(meta)) + expect_snapshot(sd_parse_crs(meta)) }) test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { @@ -39,24 +39,24 @@ test_that("sd_parse_crs works for Engineering CRS (no EPSG ID)", { } } }' - expect_snapshot(sedonadb:::sd_parse_crs(meta)) + expect_snapshot(sd_parse_crs(meta)) }) test_that("sd_parse_crs returns NULL if crs field is missing", { - expect_snapshot(sedonadb:::sd_parse_crs('{"something_else": 123}')) - expect_snapshot(sedonadb:::sd_parse_crs('{}')) + expect_snapshot(sd_parse_crs('{"something_else": 123}')) + expect_snapshot(sd_parse_crs('{}')) }) test_that("sd_parse_crs handles invalid JSON gracefully", { expect_snapshot( - sedonadb:::sd_parse_crs('invalid json'), + sd_parse_crs('invalid json'), error = TRUE ) }) test_that("sd_parse_crs works with plain strings if that's what's in 'crs'", { meta <- '{"crs": "EPSG:4326"}' - expect_snapshot(sedonadb:::sd_parse_crs(meta)) + expect_snapshot(sd_parse_crs(meta)) }) # Tests for CRS display in print.sedonadb_dataframe @@ -104,14 +104,14 @@ test_that("print.sedonadb_dataframe respects width parameter for geometry line", test_that("sd_parse_crs handles NULL input", { expect_error( - sedonadb:::sd_parse_crs(NULL), + sd_parse_crs(NULL), "must be character" ) }) test_that("sd_parse_crs handles empty string", { expect_snapshot( - sedonadb:::sd_parse_crs(""), + sd_parse_crs(""), error = TRUE ) }) @@ -123,14 +123,14 @@ test_that("sd_parse_crs handles CRS with only name, no ID", { "name": "Custom Geographic CRS" } }' - expect_snapshot(sedonadb:::sd_parse_crs(meta)) + expect_snapshot(sd_parse_crs(meta)) }) test_that("sd_parse_crs handles OGC:CRS84", { # Common case in GeoParquet/GeoArrow meta <- '{"crs": "OGC:CRS84"}' - expect_snapshot(sedonadb:::sd_parse_crs(meta)) + expect_snapshot(sd_parse_crs(meta)) }) # Explicit tests for Rust wrappers to ensure uppercase casing @@ -138,11 +138,11 @@ test_that("sd_parse_crs handles OGC:CRS84", { test_that("SedonaTypeR$crs_display() uses uppercase authority codes", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") schema <- nanoarrow::infer_nanoarrow_schema(df) - sd_type <- sedonadb:::SedonaTypeR$new(schema$children$geom) + sd_type <- SedonaTypeR$new(schema$children$geom) expect_snapshot(sd_type$crs_display()) df5070 <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 5070) as geom") - sd_type5070 <- sedonadb:::SedonaTypeR$new( + sd_type5070 <- SedonaTypeR$new( nanoarrow::infer_nanoarrow_schema(df5070)$children$geom ) expect_snapshot(sd_type5070$crs_display()) @@ -150,7 +150,7 @@ test_that("SedonaTypeR$crs_display() uses uppercase authority codes", { test_that("SedonaCrsR$display() uses uppercase authority codes", { df <- sd_sql("SELECT ST_SetSRID(ST_Point(1, 2), 4326) as geom") - sd_type <- sedonadb:::SedonaTypeR$new( + sd_type <- SedonaTypeR$new( nanoarrow::infer_nanoarrow_schema(df)$children$geom ) crs <- sd_type$crs()