yutannihilation commented on code in PR #504:
URL: https://github.com/apache/sedona-db/pull/504#discussion_r2684198337


##########
rust/sedona-functions/src/st_affine_helpers.rs:
##########
@@ -0,0 +1,531 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+use arrow_array::types::Float64Type;
+use arrow_array::Array;
+use arrow_array::PrimitiveArray;
+use datafusion_common::cast::as_float64_array;
+use datafusion_common::error::Result;
+use datafusion_common::exec_err;
+use datafusion_common::internal_err;
+use datafusion_common::DataFusionError;
+use geo_traits::{
+    CoordTrait, GeometryCollectionTrait as _, GeometryTrait, LineStringTrait,
+    MultiLineStringTrait as _, MultiPointTrait as _, MultiPolygonTrait as _, 
PointTrait,
+    PolygonTrait as _,
+};
+use sedona_geometry::wkb_factory::{
+    write_wkb_coord, write_wkb_empty_point, 
write_wkb_geometrycollection_header,
+    write_wkb_linestring_header, write_wkb_multilinestring_header, 
write_wkb_multipoint_header,
+    write_wkb_multipolygon_header, write_wkb_point_header, 
write_wkb_polygon_header,
+    write_wkb_polygon_ring_header,
+};
+use std::io::Write;
+use std::sync::Arc;
+
+pub(crate) fn invoke_affine(
+    geom: &impl GeometryTrait<T = f64>,
+    writer: &mut impl Write,
+    mat: &DAffine,
+    dim: &geo_traits::Dimensions,
+) -> Result<()> {
+    let dims = geom.dim();
+    match geom.as_type() {
+        geo_traits::GeometryType::Point(pt) => {
+            if pt.coord().is_some() {
+                write_wkb_point_header(writer, dims)
+                    .map_err(|e| DataFusionError::Execution(e.to_string()))?;
+                write_transformed_coord(writer, pt.coord().unwrap(), mat, 
dim)?;
+            } else {
+                write_wkb_empty_point(writer, dims)
+                    .map_err(|e| DataFusionError::Execution(e.to_string()))?;
+            }
+        }
+

Review Comment:
   Sounds good to me. I'll try.
   
   (As translation can also be handled by affine transformation, I was 
wondering if ST_Transform can also be implemented using glam. But, I guess 
there will be not much difference)



##########
rust/sedona-functions/src/st_affine.rs:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+use arrow_array::{builder::BinaryBuilder, Array};
+use arrow_schema::DataType;
+use datafusion_common::error::Result;
+use datafusion_expr::{
+    scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, 
Volatility,
+};
+use geo_traits::GeometryTrait;
+use sedona_expr::{
+    item_crs::ItemCrsKernel,
+    scalar_udf::{SedonaScalarKernel, SedonaScalarUDF},
+};
+use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES;
+use sedona_schema::{
+    datatypes::{SedonaType, WKB_GEOMETRY},
+    matchers::ArgMatcher,
+};
+use std::sync::Arc;
+
+use crate::{executor::WkbExecutor, st_affine_helpers};
+
+/// ST_Affine() scalar UDF
+///
+/// Native implementation for affine transformation
+pub fn st_affine_udf() -> SedonaScalarUDF {
+    SedonaScalarUDF::new(
+        "st_affine",
+        ItemCrsKernel::wrap_impl(vec![
+            Arc::new(STAffine { is_3d: true }),
+            Arc::new(STAffine { is_3d: false }),
+        ]),
+        Volatility::Immutable,
+        Some(st_affine_doc()),
+    )
+}
+
+fn st_affine_doc() -> Documentation {
+    Documentation::builder(
+        DOC_SECTION_OTHER,
+        "Apply an affine transformation to the given geometry.",
+        "ST_Affine (geom: Geometry, a: Double, b: Double, c: Double, d: 
Double, e: Double, f: Double, g: Double, h: Double, i: Double, xOff: Double, 
yOff: Double, zOff: Double)",
+    )
+    .with_argument("geom", "geometry: Input geometry")
+    .with_argument("a", "a component of the affine matrix")
+    .with_argument("b", "a component of the affine matrix")
+    .with_argument("c", "a component of the affine matrix")
+    .with_argument("d", "a component of the affine matrix")
+    .with_argument("e", "a component of the affine matrix")
+    .with_argument("f", "a component of the affine matrix")
+    .with_argument("g", "a component of the affine matrix")
+    .with_argument("h", "a component of the affine matrix")
+    .with_argument("i", "a component of the affine matrix")
+    .with_argument("xOff", "X offset")
+    .with_argument("yOff", "Y offset")
+    .with_argument("zOff", "Z offset")
+    .with_sql_example("SELECT ST_Affine(ST_GeomFromText('POLYGON Z ((1 0 1, 1 
1 1, 2 2 2, 1 0 1))'), 1, 2, 4, 1, 1, 2, 3, 2, 5, 4, 8, 3)")
+    .build()
+}
+
+#[derive(Debug)]
+struct STAffine {
+    is_3d: bool,
+}
+
+impl SedonaScalarKernel for STAffine {
+    fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+        let arg_matchers = if self.is_3d {
+            vec![
+                ArgMatcher::is_geometry(),
+                ArgMatcher::is_numeric(), // a
+                ArgMatcher::is_numeric(), // b
+                ArgMatcher::is_numeric(), // c
+                ArgMatcher::is_numeric(), // d
+                ArgMatcher::is_numeric(), // e
+                ArgMatcher::is_numeric(), // f
+                ArgMatcher::is_numeric(), // g
+                ArgMatcher::is_numeric(), // h
+                ArgMatcher::is_numeric(), // i
+                ArgMatcher::is_numeric(), // xOff
+                ArgMatcher::is_numeric(), // yOff
+                ArgMatcher::is_numeric(), // zOff
+            ]
+        } else {
+            vec![
+                ArgMatcher::is_geometry(),
+                ArgMatcher::is_numeric(), // a
+                ArgMatcher::is_numeric(), // b
+                ArgMatcher::is_numeric(), // d
+                ArgMatcher::is_numeric(), // e
+                ArgMatcher::is_numeric(), // xOff
+                ArgMatcher::is_numeric(), // yOff
+            ]
+        };
+
+        let matcher = ArgMatcher::new(arg_matchers, WKB_GEOMETRY);
+
+        matcher.match_args(args)
+    }
+
+    fn invoke_batch(
+        &self,
+        arg_types: &[SedonaType],
+        args: &[ColumnarValue],
+    ) -> Result<ColumnarValue> {
+        let executor = WkbExecutor::new(arg_types, args);
+        let mut builder = BinaryBuilder::with_capacity(
+            executor.num_iterations(),
+            WKB_MIN_PROBABLE_BYTES * executor.num_iterations(),

Review Comment:
   Ah, true. Thanks for catching.



##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -189,6 +189,177 @@ def test_st_azimuth(eng, geom1, geom2, expected):
     )
 
 
+# fmt: off
[email protected]("eng", [SedonaDB, PostGIS])
[email protected](
+    ("geom", "a", "b", "d", "e", "xoff", "yoff", "expected"),
+    [
+        (
+            "POINT (1 2)",
+            1.0, 0.0,
+            0.0, 1.0,
+            0.0, 0.0,
+            "POINT (1 2)"),
+        (
+            "POINT (1 2)",
+            2.0, 0.0,
+            0.0, 2.0,
+            1.0, 3.0,
+            "POINT (3 7)"),
+        (
+            "LINESTRING (0 0, 1 1)",
+            1.0, 0.0,
+            0.0, 1.0,
+            1.0, 2.0,
+            "LINESTRING (1 2, 2 3)"
+        ),
+    ],
+)
+def test_st_affine_2d(eng, geom, a, b, d, e, xoff, yoff, expected):
+    eng = eng.create_or_skip()
+    eng.assert_query_result(
+        "SELECT ST_Affine("
+        f"{geom_or_null(geom)}, "
+        f"{val_or_null(a)}, {val_or_null(b)}, {val_or_null(d)}, 
{val_or_null(e)}, "
+        f"{val_or_null(xoff)}, {val_or_null(yoff)})",
+        expected,
+    )
+# fmt: on
+
+
+# fmt: off
[email protected]("eng", [SedonaDB, PostGIS])
[email protected](
+    ("geom", "a", "b", "c", "d", "e", "f", "g", "h", "i", "xoff", "yoff", 
"zoff", "expected"),
+    [
+        (
+            "POINT Z (1 2 3)",
+            1.0, 0.0, 0.0,
+            0.0, 1.0, 0.0,
+            0.0, 0.0, 1.0,
+            0.0, 0.0, 0.0,
+            "POINT Z (1 2 3)",
+        ),
+        (
+            "POINT Z (1 2 3)",
+            2.0, 0.0, 0.0,
+            0.0, 2.0, 0.0,
+            0.0, 0.0, 2.0,
+            1.0, 3.0, 5.0,
+            "POINT Z (3 7 11)",
+        ),
+    ],
+)
+def test_st_affine_3d(
+    eng, geom, a, b, c, d, e, f, g, h, i, xoff, yoff, zoff, expected
+):
+    eng = eng.create_or_skip()
+    query = (
+        "SELECT ST_Affine("
+        f"{geom_or_null(geom)}, "
+        f"{val_or_null(a)}, {val_or_null(b)}, {val_or_null(c)}, "
+        f"{val_or_null(d)}, {val_or_null(e)}, {val_or_null(f)}, "
+        f"{val_or_null(g)}, {val_or_null(h)}, {val_or_null(i)}, "
+        f"{val_or_null(xoff)}, {val_or_null(yoff)}, {val_or_null(zoff)})"
+    )
+    try:
+        eng.assert_query_result(query, expected)
+    except Exception as exc:
+        if isinstance(eng, PostGIS):
+            pytest.skip(f"PostGIS may not support 3D ST_Affine: {exc}")
+        raise

Review Comment:
   Ah, sorry, these test code is generated by Codex, and it added this kind of 
skips for unknown reason. I removed it, but it seems I forgot here...



##########
rust/sedona-functions/src/st_affine.rs:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+use arrow_array::{builder::BinaryBuilder, Array};
+use arrow_schema::DataType;
+use datafusion_common::error::Result;
+use datafusion_expr::{
+    scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, 
Volatility,
+};
+use geo_traits::GeometryTrait;
+use sedona_expr::{
+    item_crs::ItemCrsKernel,
+    scalar_udf::{SedonaScalarKernel, SedonaScalarUDF},
+};
+use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES;
+use sedona_schema::{
+    datatypes::{SedonaType, WKB_GEOMETRY},
+    matchers::ArgMatcher,
+};
+use std::sync::Arc;
+
+use crate::{executor::WkbExecutor, st_affine_helpers};
+
+/// ST_Affine() scalar UDF
+///
+/// Native implementation for affine transformation
+pub fn st_affine_udf() -> SedonaScalarUDF {
+    SedonaScalarUDF::new(
+        "st_affine",
+        ItemCrsKernel::wrap_impl(vec![
+            Arc::new(STAffine { is_3d: true }),
+            Arc::new(STAffine { is_3d: false }),
+        ]),
+        Volatility::Immutable,
+        Some(st_affine_doc()),
+    )
+}
+
+fn st_affine_doc() -> Documentation {
+    Documentation::builder(
+        DOC_SECTION_OTHER,
+        "Apply an affine transformation to the given geometry.",
+        "ST_Affine (geom: Geometry, a: Double, b: Double, c: Double, d: 
Double, e: Double, f: Double, g: Double, h: Double, i: Double, xOff: Double, 
yOff: Double, zOff: Double)",
+    )
+    .with_argument("geom", "geometry: Input geometry")
+    .with_argument("a", "a component of the affine matrix")
+    .with_argument("b", "a component of the affine matrix")
+    .with_argument("c", "a component of the affine matrix")
+    .with_argument("d", "a component of the affine matrix")
+    .with_argument("e", "a component of the affine matrix")
+    .with_argument("f", "a component of the affine matrix")
+    .with_argument("g", "a component of the affine matrix")
+    .with_argument("h", "a component of the affine matrix")
+    .with_argument("i", "a component of the affine matrix")
+    .with_argument("xOff", "X offset")
+    .with_argument("yOff", "Y offset")
+    .with_argument("zOff", "Z offset")
+    .with_sql_example("SELECT ST_Affine(ST_GeomFromText('POLYGON Z ((1 0 1, 1 
1 1, 2 2 2, 1 0 1))'), 1, 2, 4, 1, 1, 2, 3, 2, 5, 4, 8, 3)")
+    .build()
+}
+
+#[derive(Debug)]
+struct STAffine {
+    is_3d: bool,
+}
+
+impl SedonaScalarKernel for STAffine {
+    fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+        let arg_matchers = if self.is_3d {
+            vec![
+                ArgMatcher::is_geometry(),
+                ArgMatcher::is_numeric(), // a
+                ArgMatcher::is_numeric(), // b
+                ArgMatcher::is_numeric(), // c
+                ArgMatcher::is_numeric(), // d
+                ArgMatcher::is_numeric(), // e
+                ArgMatcher::is_numeric(), // f
+                ArgMatcher::is_numeric(), // g
+                ArgMatcher::is_numeric(), // h
+                ArgMatcher::is_numeric(), // i
+                ArgMatcher::is_numeric(), // xOff
+                ArgMatcher::is_numeric(), // yOff
+                ArgMatcher::is_numeric(), // zOff
+            ]
+        } else {
+            vec![
+                ArgMatcher::is_geometry(),
+                ArgMatcher::is_numeric(), // a
+                ArgMatcher::is_numeric(), // b
+                ArgMatcher::is_numeric(), // d
+                ArgMatcher::is_numeric(), // e
+                ArgMatcher::is_numeric(), // xOff
+                ArgMatcher::is_numeric(), // yOff
+            ]
+        };
+
+        let matcher = ArgMatcher::new(arg_matchers, WKB_GEOMETRY);
+
+        matcher.match_args(args)
+    }
+
+    fn invoke_batch(
+        &self,
+        arg_types: &[SedonaType],
+        args: &[ColumnarValue],
+    ) -> Result<ColumnarValue> {
+        let executor = WkbExecutor::new(arg_types, args);
+        let mut builder = BinaryBuilder::with_capacity(
+            executor.num_iterations(),
+            WKB_MIN_PROBABLE_BYTES * executor.num_iterations(),
+        );
+
+        let array_args = args[1..]
+            .iter()
+            .map(|arg| {
+                arg.cast_to(&DataType::Float64, None)?
+                    .to_array(executor.num_iterations())
+            })
+            .collect::<Result<Vec<Arc<dyn Array>>>>()?;

Review Comment:
   This part, I just followed what `ST_Translate` does. Do you mean this should 
be handled differently when all the input arguments are scalars? If all are 
scalars, I think I can create a `DAffine2` / `DAffine3` here immediately 
without expanding.
   
   
https://github.com/apache/sedona-db/blob/9bf02f474d475b61a444e561ec74df449413f46b/rust/sedona-functions/src/st_translate.rs#L92-L99



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