You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2026/07/08 22:40:34 UTC
[PR] fix(rust/sedona-functions): Fix scalar iteration with num_iterations > 1 [sedona-db]
paleolimbot opened a new pull request, #1041:
URL: https://github.com/apache/sedona-db/pull/1041
This PR fixes scalar function calls with scalar geometry values but non-scalar "other" arguments, like ST_Buffer(scalar_geom, col_dist) or ST_DWithin(scalar_geom, scalar_geom, col_dist). These calls are less common so this was never caught.
The PR notably updates the executor so that it iterates over references and not values. The only place that had exploited this was the spatial join, where it was relatively easy to just add in the call to `read_wkb()` required to avoid the copy.
Most of the files here are a one line change updating reference usage.
Fixes #1038
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-functions): Fix scalar iteration with num_iterations > 1 [sedona-db]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1041:
URL: https://github.com/apache/sedona-db/pull/1041#discussion_r3552700903
##########
rust/sedona-spatial-join/src/operand_evaluator.rs:
##########
@@ -183,22 +185,30 @@ impl EvaluatedGeometryArray {
let mut wkbs = Vec::with_capacity(num_rows);
let mut x = Interval::empty();
let mut y = Interval::empty();
- geometry_array.iter_as_wkb(sedona_type, num_rows, |wkb_opt| {
- if let Some(wkb) = &wkb_opt {
+ geometry_array.iter_as_wkb_bytes(sedona_type, num_rows, |wkb_opt| {
+ if let Some(wkb_bytes) = &wkb_opt {
+ let wkb = wkb::reader::read_wkb(wkb_bytes)
+ .map_err(|e| exec_datafusion_err!("WKB parse failed: {e}"))?;
x = Interval::empty();
y = Interval::empty();
- geo_traits_update_xy_bounds(wkb, &mut x, &mut y)
+ geo_traits_update_xy_bounds(&wkb, &mut x, &mut y)
.map_err(|e| sedona_internal_datafusion_err!("{e}"))?;
rect_vec.push(Bounds2D::new(x, y));
+
+ // Safety: The wkbs must reference buffers inside the `geometry_array`. Since the `geometry_array` and
+ // `wkbs` are both owned by the `EvaluatedGeometryArray`, so they have the same lifetime. We'll never
+ // have a situation where the `EvaluatedGeometryArray` is dropped while the `wkbs` are still in use
+ // (guaranteed by the scope of the `wkbs` field and lifetime signature of the `wkbs` method).
+ wkbs.push(unsafe {
+ transmute::<Option<wkb::reader::Wkb<'_>>, Option<wkb::reader::Wkb<'_>>>(Some(
+ wkb,
+ ))
+ });
Review Comment:
The `unsafe transmute::<Option<Wkb<'_>>, Option<Wkb<'_>>>` here obscures that the code is extending the WKB lifetime to match the `Vec<Option<Wkb<'static>>>` field. Making the source/target lifetimes explicit improves reviewability and reduces the risk of accidental “no-op” transmutes in future edits (especially since CI treats clippy warnings as errors).
##########
rust/sedona-spatial-join/src/operand_evaluator.rs:
##########
@@ -223,8 +233,18 @@ impl EvaluatedGeometryArray {
// (guaranteed by the scope of the `wkbs` field and lifetime signature of the `wkbs` method).
let num_rows = geometry_array.len();
let mut wkbs = Vec::with_capacity(num_rows);
- geometry_array.iter_as_wkb(sedona_type, num_rows, |wkb_opt| {
- wkbs.push(wkb_opt.map(|wkb| unsafe { transmute(wkb) }));
+ geometry_array.iter_as_wkb_bytes(sedona_type, num_rows, |wkb_opt| {
+ if let Some(wkb_bytes) = wkb_opt {
+ let wkb = wkb::reader::read_wkb(wkb_bytes)
+ .map_err(|e| exec_datafusion_err!("WKB parse failed: {e}"))?;
+ wkbs.push(unsafe {
+ transmute::<Option<wkb::reader::Wkb<'_>>, Option<wkb::reader::Wkb<'_>>>(Some(
+ wkb,
+ ))
+ });
Review Comment:
Same as above: this transmute is performing a lifetime extension into the `Wkb<'static>` storage, but the current type parameters hide that intent. Prefer an explicit `Wkb<'_> -> Wkb<'static>` transmute (or a small helper) so the safety boundary is clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-functions): Fix scalar iteration with num_iterations > 1 [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #1041:
URL: https://github.com/apache/sedona-db/pull/1041
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org