paleolimbot commented on code in PR #275:
URL: https://github.com/apache/sedona-db/pull/275#discussion_r2491167628
##########
rust/sedona-functions/src/st_setsrid.rs:
##########
@@ -227,6 +227,79 @@ fn determine_return_type(
sedona_internal_err!("Unexpected argument types: {}, {}", args[0], args[1])
}
+#[derive(Debug)]
+pub(crate) struct SRIDifiedKernel {
Review Comment:
```suggestion
/// [SedonaScalarKernel] wrapper that handles the SRID argument for
constructors like ST_Point
#[derive(Debug)]
pub(crate) struct SRIDifiedKernel {
```
(or something better)
##########
rust/sedona-functions/src/st_point.rs:
##########
@@ -247,6 +254,23 @@ mod tests {
);
}
+ #[test]
+ fn udf_invoke_with_srid() {
+ let udf = st_point_udf();
+ let tester = ScalarUdfTester::new(
+ udf.into(),
+ vec![
+ SedonaType::Arrow(DataType::Float64),
+ SedonaType::Arrow(DataType::Float64),
+ SedonaType::Arrow(DataType::UInt16),
+ ],
+ );
+
+ tester.assert_return_type(WKB_GEOMETRY);
Review Comment:
It's a great point that the UDF tester doesn't propagate CRSes because it
doesn't consider scalar arguments. We could add
`return_type_with_with_scalars()` to the `ScalarUdfTester`?
##########
rust/sedona-functions/src/st_setsrid.rs:
##########
@@ -227,6 +227,79 @@ fn determine_return_type(
sedona_internal_err!("Unexpected argument types: {}, {}", args[0], args[1])
}
+#[derive(Debug)]
+pub(crate) struct SRIDifiedKernel {
+ inner: ScalarKernelRef,
+}
+
+impl SRIDifiedKernel {
+ pub(crate) fn new(inner: ScalarKernelRef) -> Self {
+ Self { inner }
+ }
+}
+
+impl SedonaScalarKernel for SRIDifiedKernel {
+ fn return_type_from_args_and_scalars(
+ &self,
+ args: &[SedonaType],
+ scalar_args: &[Option<&ScalarValue>],
+ ) -> Result<Option<SedonaType>> {
+ let orig_args_len = args.len() - 1;
+ let orig_args = &args[..orig_args_len];
+ let orig_scalar_args = &scalar_args[..orig_args_len];
+
+ let crs = scalar_args[orig_args_len].unwrap();
+ let new_crs = match crs.cast_to(&DataType::Utf8) {
+ Ok(ScalarValue::Utf8(Some(crs))) => {
+ if crs == "0" {
+ None
+ } else {
+ validate_crs(&crs, None)?;
+ deserialize_crs(&serde_json::Value::String(crs))?
+ }
+ }
+ Ok(ScalarValue::Utf8(None)) => None,
+ Ok(_) => {
+ // TODO: This branch is not really the "invalid CRS value"
case.
+ // If it can be cast to Utf-8, it falls into the first
branch.
+ return sedona_internal_err!("Invalid CRS value");
Review Comment:
Maybe `Can't cast Crs {crs:?} to Utf8`?
--
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]
