Implement ST_Dump #269

yutannihilation · 2025-11-01T07:03:46Z

Note: Sedona's ST_Dump returns a different result from PostGIS and DuckDB. This pull request implements the PostGIS's version ([]Struct { path: [int], geom: GEOMETRY })

https://sedona.apache.org/latest/api/sql/Function/#st_dump
https://postgis.net/docs/en/ST_Dump.html

paleolimbot

Awesome!

This crashes R session, so probably I'm doing something wrong.

When the R session crashes, please assume that I am the one that caused it 🙂 . This one is probably nanoarrow having a hard time converting an list of an extension type. Try |> as_nanoarrow_array_stream() |> as_arrow_table() (the conversion of geoarrow.wkb to extension type attempts to do a zero-copy conversion and that is probably doing something very fishy).

rust/sedona-functions/src/st_dump.rs

paleolimbot · 2025-11-01T15:35:51Z

rust/sedona-functions/src/st_dump.rs

+            // TODO: what's the decent default value for the capacity?
+            BinaryBuilder::with_capacity(8, WKB_MIN_PROBABLE_BYTES * 8),


WKB_MIN_PROBABLE_BYTES * executor.num_iterations() is probably fine. This will be very close to the input binary array in capacity and size.

paleolimbot · 2025-11-01T15:40:46Z

rust/sedona-functions/src/st_dump.rs

+
+        let input = create_array(&[Some("POINT (1 2)")], &sedona_type);
+
+        let expected = create_array(&[Some("POINT (1 2)")], &WKB_GEOMETRY);


You're right that this will be pretty verbose to test. This is a unique one but luckily byte-for-byte equality shouldn't be an issue. You can probably use create_array() here and assemble a ListArray using offsets, then use assert_eq!(). The failure message will be terrible but it will get the job done.

yutannihilation · 2025-11-01T16:24:48Z

Thanks so much for the tip!

Co-authored-by: Dewey Dunnington <[email protected]>

…ona-db into feat/st_dump

yutannihilation · 2025-11-02T00:41:03Z

Thanks to your help, I think I figured out how to handle and test these complex types. I'll work on the actual implementation...

paleolimbot

I took a stab at a Python test since I think this is going to be a pain. Feel free to take or leave, but

@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
def test_st_dump(eng):
    wkt = "MULTIPOINT (0 1, 2 3)"
    expected = [
        {
            "path": [1],
            "geom": shapely.from_wkt("POINT (0 1)").wkb,
        },
        {
            "path": [2],
            "geom": shapely.from_wkt("POINT (1 2)").wkb,
        },
    ]

    if eng == PostGIS:
        eng = eng.create_or_skip()
        result = eng.execute_and_collect(f"SELECT ST_Dump({geom_or_null(wkt)})")
    else:
        eng = eng.create_or_skip()
        result = eng.execute_and_collect(f"SELECT unnest(ST_Dump({geom_or_null(wkt)}))")
    df = eng.result_to_pandas(result)
    actual = df.iloc[0]

    for item in actual:
        assert list(item.keys()) == ["path", "geom"]

...can hopefully get you started if you haven't gotten there yet.

paleolimbot · 2025-11-03T02:48:06Z

rust/sedona-functions/src/st_dump.rs

+            if let Some(wkb) = maybe_wkb {
+                let mut struct_builder = builder.struct_builder();
+
+                let mut cur_path: Vec<u32> = Vec::new();


It probably doesn't make much of a difference here but you could declare this outside the loop and reset the size to zero to reuse any previous heap allocation.

yutannihilation · 2025-11-03T04:04:23Z

Thanks for the Python test! I hope I can get started soon with it.

yutannihilation · 2025-11-04T12:29:49Z

Benchmark result:

------------------------------------------------------------------------------------------------------------- benchmark: 6 tests -------------------------------------------------------------------------------------------------------------
Name (time in ms)                                                  Min                    Max                   Mean              StdDev                 Median                 IQR            Outliers      OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_st_dump[collections_simple-SedonaDBSingleThread]          25.8595 (1.0)          28.0133 (1.0)          26.1220 (1.0)        0.4491 (1.0)          25.9570 (1.0)        0.1710 (1.0)           2;5  38.2819 (1.0)          26           1
test_st_dump[collections_simple-DuckDBSingleThread]            54.1694 (2.09)         61.4570 (2.19)         56.3276 (2.16)       2.9164 (6.49)         55.2865 (2.13)       2.1671 (12.68)         1;1  17.7533 (0.46)          5           1
test_st_dump[collections_complex-SedonaDBSingleThread]        382.6457 (14.80)       472.4213 (16.86)       402.0678 (15.39)     39.3457 (87.60)       385.3617 (14.85)     23.3447 (136.55)        1;1   2.4871 (0.06)          5           1
test_st_dump[collections_complex-DuckDBSingleThread]          749.3529 (28.98)       763.6902 (27.26)       757.9570 (29.02)      5.9036 (13.14)       760.3898 (29.29)      9.0438 (52.90)         1;0   1.3193 (0.03)          5           1
test_st_dump[collections_simple-PostGISSingleThread]        3,297.8760 (127.53)    3,372.9212 (120.40)    3,340.3870 (127.88)    32.3133 (71.94)     3,355.1089 (129.26)    53.6658 (313.91)        2;0   0.2994 (0.01)          5           1
test_st_dump[collections_complex-PostGISSingleThread]      37,008.9275 (>1000.0)  37,411.0606 (>1000.0)  37,149.3238 (>1000.0)  184.6306 (411.07)   37,029.6383 (>1000.0)  295.5638 (>1000.0)       1;0   0.0269 (0.00)          5           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

paleolimbot

This is so cool!

One optional suggestion that you should feel free to punt on (this is also great as is!)

paleolimbot · 2025-11-04T17:11:39Z

rust/sedona-functions/src/st_dump.rs

+        let path_builder = self
+            .struct_builder
+            .field_builder::<ListBuilder<UInt32Builder>>(0)
+            .unwrap();


I am not sure it matters here given that you've nicely demonstrated that this is pretty fast, but we have tried in other places to avoid downcasting in a loop. Something like:

struct STDumpBuilder { path_array_builder: UInt32Builder, path_array_offsets_builder: OffsetBufferBuilder, geom_builder: BinaryBuilder, }

...and then implement a finish() -> StructArray that assembles at the end.

(optional!)

yutannihilation · 2025-11-04T23:40:23Z

Thanks, I simply didn't know! It looks cleaner. I'm trying to switch to that approach, but probably it takes some more time. Currently, I'm struggling to figure out how append_null() should be handled, but probably there are several more things that I still don't understand...

yutannihilation · 2025-11-05T15:52:49Z

This should be ready for review again.

paleolimbot

Thank you! (This is probably one of the hardest functions to implement 🚀 )

I have a tiny comment about CRS propagation but I'll follow-up with a PR for that one 🙂

paleolimbot · 2025-11-05T19:47:15Z

rust/sedona-functions/src/st_dump.rs

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


I believe this should to propagate the CRS of args[0] into the geometry_dump_type(). The reason this isn't done in any other type matcher code is because the type matcher does this automatically.

Ah, thanks for catching!

yutannihilation added 2 commits November 1, 2025 15:55

WIP

a33c48b

Fix clippy warning

1a66e6c

paleolimbot reviewed Nov 1, 2025

View reviewed changes

yutannihilation and others added 7 commits November 2, 2025 01:32

Apply suggestions from code review

d400c3e

Co-authored-by: Dewey Dunnington <[email protected]>

Merge branch 'feat/st_dump' of https://github.com/yutannihilation/sed…

5b5cb00

…ona-db into feat/st_dump

Use executor.num_iterations() for capacity

8f40443

Return { path: [int], geom: GEOMETRY }

a78fbe2

Fix return type

ea79541

Fix bug and add test

2e99f43

Add test util

f68e553

yutannihilation added 5 commits November 2, 2025 10:25

Wrap with builder

fd0bacf

Implement

be35ff9

Do not use raw

28c1cf7

Tweak

ec25bf5

Reject unsupported types

c9187a9

yutannihilation marked this pull request as ready for review November 2, 2025 13:31

yutannihilation added 2 commits November 3, 2025 01:19

Fix clippy warning

df817b6

Use u32 instead of i64

709da97

paleolimbot reviewed Nov 3, 2025

View reviewed changes

yutannihilation added 3 commits November 3, 2025 12:47

Merge remote-tracking branch 'upstream/main' into feat/st_dump

5e2dedc

Add Python test

485064c

Reuse Vec to avoid allocation

a1a9049

Tweak

955baf6

yutannihilation mentioned this pull request Nov 3, 2025

fix: Specify the end index in Wkb::buf() georust/wkb#87

Open

3 tasks

yutannihilation added 2 commits November 4, 2025 01:58

Add Python test

1f4818c

Add benchmark

6c29d3d

Fix clippy and typo

c16a294

paleolimbot approved these changes Nov 4, 2025

View reviewed changes

Remove STDumpStructBuilder

8936ddd

yutannihilation marked this pull request as draft November 4, 2025 23:36

yutannihilation added 2 commits November 5, 2025 23:45

Fix clippy warning

ff760ba

Use NullBuffer

07874ac

paleolimbot marked this pull request as ready for review November 5, 2025 15:44

paleolimbot marked this pull request as draft November 5, 2025 15:44

Clean up

6de1952

yutannihilation marked this pull request as ready for review November 5, 2025 15:52

paleolimbot reviewed Nov 5, 2025

View reviewed changes

paleolimbot merged commit 2939063 into apache:main Nov 5, 2025
12 checks passed

paleolimbot mentioned this pull request Nov 5, 2025

ST_Dump child geometry column should propagate CRS #278

Open

yutannihilation deleted the feat/st_dump branch November 5, 2025 23:48

		// TODO: what's the decent default value for the capacity?
		BinaryBuilder::with_capacity(8, WKB_MIN_PROBABLE_BYTES * 8),


		let input = create_array(&[Some("POINT (1 2)")], &sedona_type);

		let expected = create_array(&[Some("POINT (1 2)")], &WKB_GEOMETRY);

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

Implement ST_Dump #269

Implement ST_Dump #269

Conversation

yutannihilation commented Nov 1, 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!

Uh oh!

Uh oh!

paleolimbot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

paleolimbot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

yutannihilation commented Nov 1, 2025

Uh oh!

yutannihilation commented Nov 2, 2025

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

yutannihilation commented Nov 3, 2025

Uh oh!

yutannihilation commented Nov 4, 2025

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

yutannihilation commented Nov 4, 2025

Uh oh!

yutannihilation commented Nov 5, 2025

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

yutannihilation Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yutannihilation commented Nov 1, 2025 •

edited

Loading