petern48 commented on code in PR #171:
URL: https://github.com/apache/sedona-db/pull/171#discussion_r2403777116


##########
rust/sedona-functions/src/st_haszm.rs:
##########
@@ -107,28 +107,63 @@ impl SedonaScalarKernel for STHasZm {
     }
 }
 
-fn invoke_scalar(item: &Wkb, dim_index: usize) -> Result<Option<bool>> {
-    match item.as_type() {
-        geo_traits::GeometryType::GeometryCollection(collection) => {
-            use geo_traits::GeometryCollectionTrait;
-            if collection.num_geometries() == 0 {
-                Ok(Some(false))
-            } else {
-                // PostGIS doesn't allow creating a GeometryCollection with 
geometries of different dimensions
-                // so we can just check the dimension of the first one
-                let first_geom = unsafe { collection.geometry_unchecked(0) };
-                invoke_scalar(first_geom, dim_index)
-            }
-        }
-        _ => {
-            let geom_dim = item.dim();
-            match dim_index {
-                2 => Ok(Some(matches!(geom_dim, Dimensions::Xyz | 
Dimensions::Xyzm))),
-                3 => Ok(Some(matches!(geom_dim, Dimensions::Xym | 
Dimensions::Xyzm))),
-                _ => sedona_internal_err!("unexpected dim_index"),
-            }
+/// Fast-path inference of geometry type name from raw WKB bytes
+/// An error will be thrown for invalid WKB bytes input
+///
+/// Spec: https://libgeos.org/specifications/wkb/
+fn infer_haszm(buf: &[u8], dim_index: usize) -> Result<Option<bool>> {
+    if buf.len() < 5 {
+        return sedona_internal_err!("Invalid WKB: buffer too small ({} 
bytes)", buf.len());
+    }

Review Comment:
   Done. Let me know what you think about the approach. I'll do a follow-up PR 
to make `st_geometrytype` use this, since I have another small feature to 
implement with it. Keep in mind that the dimension() calculation can 
potentially recurse a lot (e.g a bunch of nested `GEOMETRYCOLLECTION`s), so I'd 
like to avoid just computing all of the fields at construction and saving them 
as fields. I instead went the route of computing them lazily and using the 
fields for caching values after they are calculated.



-- 
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]

Reply via email to