zhangfengcdt commented on code in PR #33: URL: https://github.com/apache/sedona-db/pull/33#discussion_r2330466702
########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,155 @@ +// 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 std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use crate::centroid::extract_centroid_2d; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], 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(), 1024); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + let centroid_wkb = invoke_scalar(&wkb)?; + builder.append_value(¢roid_wkb); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<Vec<u8>> { + let (x, y) = extract_centroid_2d(wkb)?; Review Comment: Yes, good points about the challenges with 3D centroid calculations. The geo crate currently does NOT support Z or M dimensions, similar to how GEOS (the C++ library) also focuses on 2D operations. The geo crate U=uses Coord<T> with only x and y fields - purely 2D. So, if we were to add 3D support, it would require significant effort to add new data structures with Coord3D<T> and add Z-interpolation weighted by segment length and validate against other geo libraries. What I recommend is, for now, the geo crate maintains its focus on robust 2D operations. Adding 3D support would be a major architectural change that we need to design so that sedona-db supports them consistently with different engines. We would of course need to document the behavior for whatever we support or implement. What do you think on this? @Kontinuation @jiayuasu -- 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]
