petern48 commented on code in PR #476:
URL: https://github.com/apache/sedona-db/pull/476#discussion_r2656893368
##########
c/sedona-geos/src/st_unaryunion.rs:
##########
@@ -77,7 +78,30 @@ fn invoke_scalar(geos_geom: &geos::Geometry) ->
Result<Vec<u8>> {
.to_wkb()
.map_err(|e| DataFusionError::Execution(format!("Failed to convert to
wkb: {e}")))?;
- Ok(wkb)
+ // writer.write_all(wkb.as_ref())?;
+
+ // proper z and m values
+ let (has_z, has_m) = (geometry.has_z().unwrap(),
geometry.has_m().unwrap());
+ println!("before to_wkb: has_z: {:?}, has_m: {:?}", has_z, has_m);
+
+ // results when using .to_wkb(): drops z and m values
+ let tmp_geom = geos::Geometry::new_from_wkb(&wkb).unwrap();
+ let (has_z, has_m) = (tmp_geom.has_z().unwrap(),
tmp_geom.has_m().unwrap());
+ println!("after to_wkb(): has_z: {:?}, has_m: {:?}", has_z, has_m); //
dropped z and m values
+
+ // wkb_writer method maintains proper z and m values
+ use geos::WKBWriter;
+ let mut wkb_writer = WKBWriter::new().unwrap();
+ // use geos::OutputDimension;
+ // wkb_writer.set_output_dimension(if has_z && has_m {
OutputDimension::FourD } else if has_z { OutputDimension::ThreeD } else if
has_m { OutputDimension::ThreeD } else { OutputDimension::TwoD });
+ let wkb = wkb_writer.write_wkb(&geometry).unwrap();
+ let tmp_geom = geos::Geometry::new_from_wkb(&wkb).unwrap();
+ let (has_z, has_m) = (tmp_geom.has_z().unwrap(),
tmp_geom.has_m().unwrap());
+ println!("after write_wkb(): has_z: {:?}, has_m: {:?}", has_z, has_m); //
proper z and m values
Review Comment:
Side note: I sketched out some code here that led to some interesting
behavior (I'm immediately deleting it, but I'll comment here to save my
thoughts somewhere. I found that `.to_wkb()` drops the Z and M dimensions.
After looking into it, I see `.to_wkb()` is [implemented using
GEOSGeomToWKB_buf_r](https://github.com/georust/geos/blob/47afbad2483e489911ddb456417808340e9342c3/src/geometry.rs#L2229),
which [is deprecated and suggestes using GEOSWKBWriter_write
instead](https://github.com/libgeos/geos/blob/158c588566e08fd2f31f8c9e7ae069518e8264c5/capi/geos_c.h.in#L6685).
In GeoRust, that cooresponds to using
[WKBWriter.write_wkb()](https://github.com/georust/geos/blob/47afbad2483e489911ddb456417808340e9342c3/src/wkb_writer.rs#L71)
instead. I tried it out, and it seems to work. tldr; `WKBWriter.write_wkb`
should be used instead of `.to_wkb()`.
This code used on `POINT ZM (1 2 3 4)` results in the following.
```
before to_wkb: has_z: true, has_m: true
after to_wkb(): has_z: false, has_m: false
after write_wkb(): has_z: true, has_m: true
```
Though in reality, I think this new method `write_geos_geometry` should
replace any need for `write_wkb()`.
--
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]
