feat(rust/sedona-geo): Add ST_AsGeoJSON #469

prantogg · 2025-12-28T18:33:44Z

Implements the ST_AsGeoJSON function for converting geometry objects to GeoJSON format.

paleolimbot

Thank you for working on this!

From CI I see:

error: use of a fallible conversion when an infallible one could be used
   --> rust/sedona-functions/src/st_asgeojson.rs:179:71
    |
179 |                         let geojson_geom: geojson::Geometry = (&geom).try_into().map_err(|e| {
    |                                                                       ^^^^^^^^ help: use: `into`
    |
    = note: converting `&Geometry` to `Geometry` cannot fail
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#unnecessary_fallible_conversions
    = note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]`

You should be able to run cargo clippy --fix and I think it will fix it for you.

This needs Python integration tests in python/sedonadb/tests (I think there is a file for one of the IO functions). The Python integration tests check against PostGIS, and you can follow the examples from the other functions with respect to which things to test (usually: all the empties, one non-empty for each geometry type).

I might suggest for the initial implementation to skip the output type parameter (i.e., get used to the testing helpers and repo structure and implement the output type parameter in a follow-up PR). I left a note about how we might make this faster using geoarrow-rs but no need to do that here.

paleolimbot · 2025-12-29T03:37:39Z

rust/sedona-functions/src/st_asgeojson.rs

+    args: &[ColumnarValue],
+    geojson_type: GeoJsonType,
+) -> Result<ColumnarValue> {
+    let executor = WkbExecutor::new(arg_types, args);


A good reason to put this in the sedona-geo crate is that there we have the GeoTypesExecutor (which will let you remove the wkb -> geo_types conversion)

+1 to moving to this from rust/sedona-functions/ to rust/sedona-geo. Note you need to do this in order to access these helpful methods/structs.

GeoTypesExecutor uses a item_to_geometry() function that implements exactly what you did in wkb_to_geometry() below (hence we can remove it). If you find GeoTypesExecutor isn't flexible enough for this function (like I did for a different function), you could consider using item_to_geometry() directly instead.

Thanks! GeoTypesExecutor is exactly what i needed!

paleolimbot · 2025-12-29T03:45:19Z

rust/sedona-functions/src/st_asgeojson.rs

+        if let ScalarValue::Utf8(Some(json_str)) = result {
+            // Verify it's valid JSON and contains expected structure
+            let parsed: serde_json::Value = serde_json::from_str(json_str.as_str()).unwrap();
+            assert_eq!(parsed["type"], "Point");
+            assert!(parsed["coordinates"].is_array());
+        } else {
+            panic!("Expected Utf8 scalar value");
+        }


Our tester helper is probably the best way to test these results:

Suggested change

if let ScalarValue::Utf8(Some(json_str)) = result {

// Verify it's valid JSON and contains expected structure

let parsed: serde_json::Value = serde_json::from_str(json_str.as_str()).unwrap();

assert_eq!(parsed["type"], "Point");

assert!(parsed["coordinates"].is_array());

} else {

panic!("Expected Utf8 scalar value");

}

tester.assert_scalar_result_equal(result, #"{"key": "value"}"#);

paleolimbot · 2025-12-29T03:48:20Z

rust/sedona-functions/src/st_asgeojson.rs

+        // Verify the array has the expected number of elements
+        assert_eq!((*result_array).len(), 3);


We probably do want to create an explicit array here and check for equality. I believe we do this for other functions that return a string (like ST_GeometryType()).

paleolimbot · 2025-12-29T03:50:05Z

rust/sedona-functions/src/st_asgeojson.rs

+impl SedonaScalarKernel for STAsGeoJSON {
+    fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+        let matcher = ArgMatcher::new(
+            vec![ArgMatcher::is_geometry_or_geography()],


Suggested change

vec![ArgMatcher::is_geometry_or_geography()],

vec![ArgMatcher::is_geometry()],

(GeoJSON has explicit planar edges, unlike geographies)

paleolimbot · 2025-12-29T03:50:18Z

rust/sedona-functions/src/st_asgeojson.rs

+    fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+        let matcher = ArgMatcher::new(
+            vec![
+                ArgMatcher::is_geometry_or_geography(),


Suggested change

ArgMatcher::is_geometry_or_geography(),

ArgMatcher::is_geometry(),

paleolimbot · 2025-12-29T03:51:09Z

rust/sedona-functions/src/st_asgeojson.rs

+                return Err(DataFusionError::Execution(
+                    "Second argument to ST_AsGeoJSON must be a string literal".to_string(),
+                ));


Suggested change

return Err(DataFusionError::Execution(

"Second argument to ST_AsGeoJSON must be a string literal".to_string(),

));

return exec_err!("Second argument to ST_AsGeoJSON must be a string literal");

paleolimbot · 2025-12-29T04:22:39Z

rust/sedona-functions/src/st_asgeojson.rs

+                // Convert WKB geometry to geo_types::Geometry using geo-traits
+                let geo_geometry = wkb_to_geometry(item)?;
+
+                match geo_geometry {
+                    Some(geom) => {
+                        // Convert geo_types::Geometry to geojson::Geometry
+                        let geojson_geom: geojson::Geometry = (&geom).try_into().map_err(|e| {
+                            DataFusionError::Execution(format!(
+                                "Failed to convert to GeoJSON: {:?}",
+                                e
+                            ))
+                        })?;


We can do this in a follow-up PR and not this one; however, this chain of WKB bytes -> Wkb object -> GeoTypes geometry -> GeoJSON geometry is probably very slow. If we can use:

https://github.com/geoarrow/geoarrow-rs/blob/e17e9e48a6e1b357d5c71839dd33c7cb72d853c0/rust/geoarrow-geojson/src/encoder/geometry.rs#L31-L48

(which should operate directly on a Wkb object), then I think we might be able to make this quite a bit faster (and support Z values).

Thanks! The current implementation still does the full chain (via GeoTypesExecutor -> geo_types::Geometry -> geojson::Geometry -> JSON string).
I'm happy to address the optimization using geoarrow-geojson's encoder in a follow-up PR if that's fine by you?

filed a ticket so we don't forget: #472

petern48 · 2025-12-29T04:41:55Z

rust/sedona-functions/src/st_asgeojson.rs

+}
+
+#[cfg(test)]
+mod tests {


In addition to rust tests, we need integration tests in test_functions.py. Instructions for testing it locally and iterating are here.

petern48 · 2025-12-29T04:53:09Z

rust/sedona-functions/src/st_asgeojson.rs

+    .with_argument(
+        "type",
+        "string (optional): Output type - 'Simple' (default), 'Feature', or 'FeatureCollection'",
+    )
+    .with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0))")
+    .with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'Feature')")
+    .with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'FeatureCollection')")


I'm not sure if you already discussed this or not, but I'll point out that this follows the Sedona API. The PostGIS one is completely different.

text ST_AsGeoJSON(record feature, text geom_column="", integer maxdecimaldigits=9, boolean pretty_bool=false, text id_column=''); text ST_AsGeoJSON(geometry geom, integer maxdecimaldigits=9, integer options=8); text ST_AsGeoJSON(geography geog, integer maxdecimaldigits=9, integer options=0);

We have generally followed the PostGIS behavior for all previous functions that I know of.

Interesting, there's also a note on the Sedona page that says

"This method is not recommended. Please use Sedona GeoJSON data source to write GeoJSON files." (bold is mine)

... haha. Maybe it's because the api is different 🤷

petern48 · 2025-12-29T05:10:59Z

rust/sedona-functions/src/st_asgeojson.rs

+    args: &[ColumnarValue],
+    geojson_type: GeoJsonType,
+) -> Result<ColumnarValue> {
+    let executor = WkbExecutor::new(arg_types, args);


+1 to moving to this from rust/sedona-functions/ to rust/sedona-geo. Note you need to do this in order to access these helpful methods/structs.

GeoTypesExecutor uses a item_to_geometry() function that implements exactly what you did in wkb_to_geometry() below (hence we can remove it). If you find GeoTypesExecutor isn't flexible enough for this function (like I did for a different function), you could consider using item_to_geometry() directly instead.

…ry types

prantogg · 2025-12-30T19:51:48Z

Thanks for the feedback! I've moved the code to rust/sedona-geo and am now using GeoTypesExecutor as suggested. I've also removed the output type parameter for now - we can address that in a later PR.

However, I'm running into some CI python test failures when comparing GeoJSON output between SedonaDB and PostGIS.
The differences arise from both the geojson crate's serialization and how it converts from geo_types::Geometry. Specifically:

Coordinate formatting (integers vs floats): The geojson crate always serializes coordinates as floats, even for whole numbers. PostGIS optimizes by omitting .0 for whole numbers.
Empty polygon representation: The geojson crate's conversion from geo_types::Polygon to geojson::Geometry represents POLYGON EMPTY as {"type":"Polygon","coordinates":[[]]} (one empty ring), while PostGIS returns {"type":"Polygon","coordinates":[]} (no rings).

What's your preference for handling these format differences? Should we:

Normalize these in the tests (converting PostGIS output to match SedonaDB's format)?
Look into customizing the GeoJSON serialization to match PostGIS exactly?

petern48

The geojson crate's conversion from geo_types::Polygon to geojson::Geometry represents POLYGON EMPTY as {"type":"Polygon","coordinates":[[]]} (one empty ring), while PostGIS returns {"type":"Polygon","coordinates":[]} (no rings).

I do think this is technically a bug upstream (see the toggle for why), and there does seem to be a maintainer responding to new isssues/PRs in the geojson rust repo. I'd say if you're eager to fix it, then go for it, but I think it's fine to kick this down the road since this is such a specific edge case. At least it's valid traditional JSON. We can just file an issue and comment the test out.

Proof from the [GeoJSON spec](https://datatracker.ietf.org/doc/html/rfc7946) (I asked an LLM and verified it 🤠)

A linear ring is a closed LineString with four or more positions.

For type "Polygon", the "coordinates" member MUST be an array of linear ring coordinate arrays.

Coordinate formatting (integers vs floats): The geojson crate always serializes coordinates as floats, even for whole numbers. PostGIS optimizes by omitting .0 for whole numbers.

I think fixing this would be convenient to do when we decide to add support for the integer maxdecimaldigits argument for ST_AsGeoJSON, which would likely involve modifying similar code upstream. Speaking of, it seems like there's an old PR that might be fixing this for us: georust/geojson#234, though I didn't look too deeply into it.

For both cases, I'm in favor of tweaking the tests a bit to pass and keeping existing behavior for convenience. We'll see what Dewey thinks.

petern48 · 2025-12-30T22:18:04Z

rust/sedona-functions/src/st_asgeojson.rs

+    .with_argument("geom", "geometry: Input geometry")
+    .with_argument(
+        "type",
+        "string (optional): Output type - 'Simple' (default), 'Feature', or 'FeatureCollection'",


Suggested change

"string (optional): Output type - 'Simple' (default), 'Feature', or 'FeatureCollection'",

(based on you removing it from the actual kernel implementation)

petern48 · 2025-12-30T22:18:20Z

rust/sedona-functions/src/st_asgeojson.rs

+    .with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'Feature')")
+    .with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'FeatureCollection')")


Suggested change

.with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'Feature')")

.with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'FeatureCollection')")

rust/sedona-geo/src/st_asgeojson.rs

Co-authored-by: Peter Nguyen <petern0408@gmail.com>

paleolimbot

Thank you for the edits, and thank you Peter for the review + follow-up!

I'm personally OK with the 1.0 vs 1 difference, although we should solve the empty polygon and empty point here.

paleolimbot · 2025-12-31T02:36:59Z

python/sedonadb/tests/functions/test_functions.py

+        ("POINT (1 2)", '{"type":"Point","coordinates":[1.0,2.0]}'),
+        (
+            "LINESTRING (0 0, 1 1)",
+            '{"type":"LineString","coordinates":[[0.0,0.0],[1.0,1.0]]}',
+        ),


For the failing tests, how about something like:

if eng is PostGIS: expected = expected.replace(".0", "")

Slightly less hacky but might not work (not sure if Python dictionary equality works exactly in the way we need it to for this):

result = eng.execute_and_collect() df = eng.result_to_pandas(result) assert df.shape == (1, 1) assert json.loads(df.iloc[0, 0]) == json.loads(expected)

Either works for me! I don't particularly mind the difference (in the process of making it faster we might be able to fix the incompatibility as well).

Alternatively, just add 0.5 to all of your coordinates (maybe add a comment with a link tp #472 or a link to this PR and a short explanation of why you aren't using any integer coordinates in your test data given that all the other tests do).

paleolimbot · 2025-12-31T02:41:22Z

python/sedonadb/tests/functions/test_functions.py

+        (None, None),
+        ("LINESTRING EMPTY", '{"type":"LineString","coordinates":[]}'),


Suggested change

(None, None),

("LINESTRING EMPTY", '{"type":"LineString","coordinates":[]}'),

(None, None),

("POINT EMPTY", '{"type":"Point","coordinates":[]}'),

("LINESTRING EMPTY", '{"type":"LineString","coordinates":[]}'),

paleolimbot · 2025-12-31T02:50:55Z

rust/sedona-geo/src/st_asgeojson.rs

+                Some(geom) => {
+                    // Convert geo_types::Geometry to geojson::Geometry
+                    let geojson_geom: geojson::Geometry = (&geom).into();


You can solve the lack of empty point support and empty polygon incompatiblity here by special casing them. Maybe:

match geom.as_type() { geo_traits::GeometryType::Point(pt) => { if pt.coord().is_none() { /* special case the empty point output and return */ } } geo_traits::GeometryType::Polygon(pl) => { if pl.exterior().is_none() { /* special case the empty polygon output and return */ } } let geo_geom = item_to_geometry(geom)?; /* current implementation */ }

This means you'd have to go back to the WkbExecutor because geo_types can't represent an empty point. Most of the functions implemented in this crate have to do that (the ones that don't aren't used by default at the moment).

Opened a quick fix for POLYGON EMPTY in geojson: georust/geojson#262. Though we should just move forward with the workaround.

paleolimbot

Thank you!

prantogg added 2 commits December 28, 2025 10:18

feat: implement ST_AsGeoJSON

c167280

clippy fix

dfba3d6

prantogg requested a review from paleolimbot December 28, 2025 18:33

fmt fix

180047b

paleolimbot reviewed Dec 29, 2025

View reviewed changes

petern48 reviewed Dec 29, 2025

View reviewed changes

prantogg added 5 commits December 29, 2025 11:58

refactor: use existing GeoTypesExecutorfor iterating through geomet…

4421184

…ry types

remove output type parameter

9294a77

add integration tests

ad886bb

remove empty point test

29f333c

pre-commit fix

3660f07

prantogg requested review from paleolimbot and petern48 December 30, 2025 19:51

petern48 reviewed Dec 30, 2025

View reviewed changes

petern48 mentioned this pull request Dec 30, 2025

Optimize ST_AsGeoJSON using encode_geometry() #472

Open

Apply @petern48's suggestions from code review

0da4fc0

Co-authored-by: Peter Nguyen <petern0408@gmail.com>

paleolimbot reviewed Dec 31, 2025

View reviewed changes

petern48 mentioned this pull request Dec 31, 2025

chore: Handle unsupported geometries (e.g. point empty) in GeoExecutor #473

Open

prantogg added 2 commits January 1, 2026 17:43

Special case empty point and polygon

5c340db

fix python tests

5b092e4

paleolimbot approved these changes Jan 2, 2026

View reviewed changes

paleolimbot changed the title ~~feat(rust/sedona-functions): Add ST_AsGeoJSON~~ feat(rust/sedona-geo): Add ST_AsGeoJSON Jan 2, 2026

prantogg merged commit 109e772 into main Jan 2, 2026
14 checks passed

prantogg deleted the pranav/add-st_asgeojson branch January 2, 2026 04:01

paleolimbot added this to the 0.3.0 milestone Jan 26, 2026

		// Verify the array has the expected number of elements
		assert_eq!((*result_array).len(), 3);

	vec![ArgMatcher::is_geometry_or_geography()],
	vec![ArgMatcher::is_geometry()],

	ArgMatcher::is_geometry_or_geography(),
	ArgMatcher::is_geometry(),

		.with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'Feature')")
		.with_sql_example("SELECT ST_AsGeoJSON(ST_Point(1.0, 2.0), 'FeatureCollection')")

		(None, None),
		("LINESTRING EMPTY", '{"type":"LineString","coordinates":[]}'),

feat(rust/sedona-geo): Add ST_AsGeoJSON #469

feat(rust/sedona-geo): Add ST_AsGeoJSON #469

Uh oh!

Conversation

prantogg commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petern48 Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prantogg commented Dec 30, 2025

Uh oh!

petern48 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prantogg commented Dec 28, 2025 •

edited

Loading

petern48 Dec 29, 2025 •

edited

Loading

petern48 left a comment •

edited

Loading