Copilot commented on code in PR #799:
URL: https://github.com/apache/sedona-db/pull/799#discussion_r3163130300
##########
c/sedona-libgpuspatial/src/lib.rs:
##########
@@ -126,7 +126,8 @@ mod sys {
}
/// Probes the spatial index with a batch of rectangles and returns
pairs of matching indices from the build and probe sets.
- pub fn probe(&self, rects: &[Rect<f32>]) -> Result<(Vec<u32>,
Vec<u32>)> {
+ /// The values in rects are ordered (xmin, ymin, xmax, ymax).
+ pub fn probe(&self, rects: &[(f32, f32, f32, f32)]) ->
Result<(Vec<u32>, Vec<u32>)> {
let raw_ptr = rects.as_ptr() as *const f32;
self.inner.probe(raw_ptr, rects.len() as u32)
}
Review Comment:
Same layout issue as `push_build`: casting `&[(f32, f32, f32, f32)]` to
`*const f32` relies on tuple layout being tightly packed, which is not
guaranteed by Rust and can cause UB. Prefer `&[[f32; 4]]` or a `#[repr(C)]`
struct for the rectangle representation before passing a raw pointer.
##########
rust/sedona-geometry/src/analyze.rs:
##########
@@ -15,44 +15,35 @@
// specific language governing permissions and limitations
// under the License.
use crate::{
- bounding_box::BoundingBox,
error::SedonaGeometryError,
- interval::IntervalTrait,
point_count::count_points,
types::{GeometryTypeAndDimensions, GeometryTypeId},
};
use wkb::reader::Wkb;
-/// Captures the size, bounds, and type-derived counts for a single geometry.
+/// Captures the size and type-derived counts for a single geometry.
/// Used as the per-geometry input that eventually feeds aggregated
`GeoStatistics`.
#[derive(Debug, Clone)]
pub struct GeometrySummary {
pub size_bytes: usize,
pub point_count: i64,
pub geometry_type: GeometryTypeAndDimensions,
- pub bbox: BoundingBox,
pub puntal_count: i64,
pub lineal_count: i64,
pub polygonal_count: i64,
pub collection_count: i64,
}
-/// Analyzes a WKB geometry and returns its size, point count, dimensions, and
type
-pub fn analyze_geometry(geom: &Wkb) -> Result<GeometrySummary,
SedonaGeometryError> {
+/// Analyzes a WKB geometry and returns its size, point count, and dimensions
+pub fn analyze_wkb(geom: &Wkb) -> Result<GeometrySummary, SedonaGeometryError>
{
// Get size in bytes directly from WKB buffer
let size_bytes = geom.buf().len();
- // Get geometry type using as_type() which is public
- let geometry_type = GeometryTypeAndDimensions::try_from_geom(geom)?;
-
// Get point count directly using the geometry traits
let point_count = count_points(geom);
- // Calculate bounding box using geo_traits_update_xy_bounds
- let mut x = crate::interval::Interval::empty();
- let mut y = crate::interval::Interval::empty();
- crate::bounds::geo_traits_update_xy_bounds(geom, &mut x, &mut y)?;
- let bbox = BoundingBox::xy(x, y);
+ // Calculate bounding box and geometry types using
geo_traits_update_xy_bounds
Review Comment:
The comment says we "Calculate bounding box ... using
geo_traits_update_xy_bounds", but `analyze_wkb()` no longer computes bounds (it
only derives `geometry_type`). Updating this comment would avoid confusion for
callers trying to understand where bbox/envelope stats come from now.
```suggestion
// Derive geometry type and dimensions from the geometry
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
