You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/06/08 15:33:33 UTC
Re: [PR] feat(rust/sedona-raster-gdal): add RS_MetaData [sedona-db]
Copilot commented on code in PR #833:
URL: https://github.com/apache/sedona-db/pull/833#discussion_r3374460217
##########
rust/sedona-raster-gdal/src/rs_metadata.rs:
##########
@@ -0,0 +1,440 @@
+// 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::{Float64Builder, Int32Builder, UInt32Builder, UInt64Builder};
+use arrow_array::StructArray;
+use arrow_schema::{DataType, Field, Fields};
+use datafusion_common::config::ConfigOptions;
+use datafusion_common::error::Result;
+use datafusion_common::exec_datafusion_err;
+use datafusion_expr::{ColumnarValue, Volatility};
+use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
+use sedona_raster::traits::RasterRef;
+use sedona_schema::crs::deserialize_crs;
+use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher};
+
+use crate::gdal_common::with_gdal;
+use crate::gdal_dataset_provider::{configure_thread_local_options, thread_local_provider};
+
+pub fn rs_metadata_udf() -> SedonaScalarUDF {
+ SedonaScalarUDF::new(
+ "rs_metadata",
+ vec![Arc::new(RsMetaData {})],
+ Volatility::Immutable,
+ )
+}
+
+fn metadata_struct_fields() -> Fields {
+ Fields::from(vec![
+ Field::new("upperLeftX", DataType::Float64, true),
+ Field::new("upperLeftY", DataType::Float64, true),
+ Field::new("gridWidth", DataType::UInt64, true),
+ Field::new("gridHeight", DataType::UInt64, true),
+ Field::new("scaleX", DataType::Float64, true),
+ Field::new("scaleY", DataType::Float64, true),
+ Field::new("skewX", DataType::Float64, true),
+ Field::new("skewY", DataType::Float64, true),
+ Field::new("srid", DataType::Int32, true),
+ Field::new("numSampleDimensions", DataType::UInt32, true),
+ Field::new("tileWidth", DataType::UInt64, true),
+ Field::new("tileHeight", DataType::UInt64, true),
+ ])
+}
+
+#[derive(Debug)]
+struct RsMetaData {}
+
+impl SedonaScalarKernel for RsMetaData {
+ fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+ let matcher = ArgMatcher::new(
+ vec![ArgMatcher::is_raster()],
+ SedonaType::Arrow(DataType::Struct(metadata_struct_fields())),
+ );
+
+ matcher.match_args(args)
+ }
+
+ fn invoke_batch(
+ &self,
+ arg_types: &[SedonaType],
+ args: &[ColumnarValue],
+ ) -> Result<ColumnarValue> {
+ self.invoke_batch_from_args(arg_types, args, &SedonaType::Arrow(DataType::Null), 0, None)
+ }
+
+ fn invoke_batch_from_args(
+ &self,
+ arg_types: &[SedonaType],
+ args: &[ColumnarValue],
+ _return_type: &SedonaType,
+ _num_rows: usize,
+ config_options: Option<&ConfigOptions>,
+ ) -> Result<ColumnarValue> {
+ let executor = sedona_raster_functions::RasterExecutor::new(arg_types, args);
+ let capacity = executor.num_iterations();
+
+ let mut upper_left_x_builder = Float64Builder::with_capacity(capacity);
+ let mut upper_left_y_builder = Float64Builder::with_capacity(capacity);
+ let mut grid_width_builder = UInt64Builder::with_capacity(capacity);
+ let mut grid_height_builder = UInt64Builder::with_capacity(capacity);
+ let mut scale_x_builder = Float64Builder::with_capacity(capacity);
+ let mut scale_y_builder = Float64Builder::with_capacity(capacity);
+ let mut skew_x_builder = Float64Builder::with_capacity(capacity);
+ let mut skew_y_builder = Float64Builder::with_capacity(capacity);
+ let mut srid_builder = Int32Builder::with_capacity(capacity);
+ let mut num_bands_builder = UInt32Builder::with_capacity(capacity);
+ let mut tile_width_builder = UInt64Builder::with_capacity(capacity);
+ let mut tile_height_builder = UInt64Builder::with_capacity(capacity);
+
+ with_gdal(|gdal| {
+ configure_thread_local_options(gdal, config_options)?;
+ let provider = thread_local_provider(gdal)
+ .map_err(|e| exec_datafusion_err!("Failed to init GDAL provider: {e}"))?;
+
+ executor.execute_raster_void(|_i, raster_opt| {
+ match raster_opt {
+ None => {
+ upper_left_x_builder.append_null();
+ upper_left_y_builder.append_null();
+ grid_width_builder.append_null();
+ grid_height_builder.append_null();
+ scale_x_builder.append_null();
+ scale_y_builder.append_null();
+ skew_x_builder.append_null();
+ skew_y_builder.append_null();
+ srid_builder.append_null();
+ num_bands_builder.append_null();
+ tile_width_builder.append_null();
+ tile_height_builder.append_null();
+ }
+ Some(raster) => {
+ let metadata = raster.metadata();
+ let num_bands = raster.bands().len() as u32;
+
+ upper_left_x_builder.append_value(metadata.upper_left_x());
+ upper_left_y_builder.append_value(metadata.upper_left_y());
+ grid_width_builder.append_value(metadata.width());
+ grid_height_builder.append_value(metadata.height());
+ scale_x_builder.append_value(metadata.scale_x());
+ scale_y_builder.append_value(metadata.scale_y());
+ skew_x_builder.append_value(metadata.skew_x());
+ skew_y_builder.append_value(metadata.skew_y());
+
+ let srid = match raster.crs() {
+ None => 0i32,
+ Some(crs_str) => match deserialize_crs(crs_str) {
+ Ok(Some(crs_ref)) => {
+ crs_ref.srid().ok().flatten().map(|s| s as i32).unwrap_or(0)
+ }
+ _ => 0i32,
+ },
+ };
+ srid_builder.append_value(srid);
+
+ num_bands_builder.append_value(num_bands);
+
+ if num_bands == 0 {
+ tile_width_builder.append_value(0);
+ tile_height_builder.append_value(0);
+ } else {
+ let dataset = provider.raster_ref_to_gdal(raster).map_err(|e| {
+ exec_datafusion_err!("Failed to create GDAL dataset: {e}")
+ })?;
+ let band1 = dataset
+ .as_dataset()
+ .rasterband(1)
+ .map_err(|e| exec_datafusion_err!("Failed to get band 1: {e}"))?;
+ let (block_x, block_y) = band1.block_size();
+ tile_width_builder.append_value(block_x as u64);
+ tile_height_builder.append_value(block_y as u64);
+ }
+ }
+ }
+ Ok(())
+ })
+ })?;
+
+ let struct_array = StructArray::new(
+ metadata_struct_fields(),
+ vec![
+ Arc::new(upper_left_x_builder.finish()),
+ Arc::new(upper_left_y_builder.finish()),
+ Arc::new(grid_width_builder.finish()),
+ Arc::new(grid_height_builder.finish()),
+ Arc::new(scale_x_builder.finish()),
+ Arc::new(scale_y_builder.finish()),
+ Arc::new(skew_x_builder.finish()),
+ Arc::new(skew_y_builder.finish()),
+ Arc::new(srid_builder.finish()),
+ Arc::new(num_bands_builder.finish()),
+ Arc::new(tile_width_builder.finish()),
+ Arc::new(tile_height_builder.finish()),
+ ],
+ None,
+ );
Review Comment:
`StructArray::new(..., None)` builds a struct with no validity bitmap, so rows for null raster inputs become *non-null structs with all-null fields* rather than a null struct. This breaks SQL null-propagation semantics (e.g., `RS_MetaData(raster_col) IS NULL` won’t be true when `raster_col` is NULL) and also affects scalar-null behavior via `ScalarValue::try_from_array()`.
Pass a struct-level null buffer (e.g., derived from one of the child arrays’ nulls, since those are already appended as nulls on null input).
##########
rust/sedona-raster-gdal/src/rs_metadata.rs:
##########
@@ -0,0 +1,440 @@
+// 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::{Float64Builder, Int32Builder, UInt32Builder, UInt64Builder};
+use arrow_array::StructArray;
+use arrow_schema::{DataType, Field, Fields};
+use datafusion_common::config::ConfigOptions;
+use datafusion_common::error::Result;
+use datafusion_common::exec_datafusion_err;
+use datafusion_expr::{ColumnarValue, Volatility};
+use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
+use sedona_raster::traits::RasterRef;
+use sedona_schema::crs::deserialize_crs;
+use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher};
+
+use crate::gdal_common::with_gdal;
+use crate::gdal_dataset_provider::{configure_thread_local_options, thread_local_provider};
+
+pub fn rs_metadata_udf() -> SedonaScalarUDF {
+ SedonaScalarUDF::new(
+ "rs_metadata",
+ vec![Arc::new(RsMetaData {})],
+ Volatility::Immutable,
+ )
Review Comment:
`rs_metadata` opens/inspects the underlying GDAL dataset (e.g., to read block size). That means results can change across queries for the same logical raster value if the external storage changes, so this shouldn’t be marked `Volatility::Immutable` (which enables optimizations like constant folding/caching under the assumption of full determinism).
Use `Volatility::Stable` (consistent with `rs_ensure_loaded`’s rationale) or `Volatility::Volatile` if you want to be maximally conservative.
##########
docs/reference/sql/rs_metadata.qmd:
##########
@@ -0,0 +1,53 @@
+---
+# 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.
+
+title: RS_MetaData
+description: Returns raster metadata as a struct.
+kernels:
+ - returns: struct
+ args: [raster]
+---
+
+## Description
+
+`RS_MetaData()` returns a struct containing:
+
+- `upperLeftX`, `upperLeftY`: origin of the raster geotransform
+- `gridWidth`, `gridHeight`: raster dimensions in pixels
+- `scaleX`, `scaleY`: pixel scale
+- `skewX`, `skewY`: geotransform skew terms
+- `srid`: raster SRID if available, otherwise `0`
+- `numSampleDimensions`: band count
+- `tileWidth`, `tileHeight`: GDAL block size derived tile dimensions
+
+For rasters with no bands, `tileWidth` and `tileHeight` are `0`.
+
+## Examples
+
+```sql
+SELECT RS_MetaData(
+ RS_FromPath('../../../submodules/sedona-testing/data/raster/test4.tiff')
+);
+```
+
+```sql
+SELECT meta."gridWidth", meta."gridHeight"
+FROM (
+ SELECT RS_MetaData(RS_FromPath('../../../submodules/sedona-testing/data/raster/test4.tiff')) AS meta
+);
Review Comment:
The second SQL example uses a derived table without an alias (`FROM ( ... );`). Many SQL engines (and DataFusion) require an alias for subqueries in `FROM`, so this example is likely to fail as written.
##########
rust/sedona-raster-gdal/src/rs_metadata.rs:
##########
@@ -0,0 +1,440 @@
+// 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::{Float64Builder, Int32Builder, UInt32Builder, UInt64Builder};
+use arrow_array::StructArray;
+use arrow_schema::{DataType, Field, Fields};
+use datafusion_common::config::ConfigOptions;
+use datafusion_common::error::Result;
+use datafusion_common::exec_datafusion_err;
+use datafusion_expr::{ColumnarValue, Volatility};
+use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
+use sedona_raster::traits::RasterRef;
+use sedona_schema::crs::deserialize_crs;
+use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher};
+
+use crate::gdal_common::with_gdal;
+use crate::gdal_dataset_provider::{configure_thread_local_options, thread_local_provider};
+
+pub fn rs_metadata_udf() -> SedonaScalarUDF {
+ SedonaScalarUDF::new(
+ "rs_metadata",
+ vec![Arc::new(RsMetaData {})],
+ Volatility::Immutable,
+ )
+}
+
+fn metadata_struct_fields() -> Fields {
+ Fields::from(vec![
+ Field::new("upperLeftX", DataType::Float64, true),
+ Field::new("upperLeftY", DataType::Float64, true),
+ Field::new("gridWidth", DataType::UInt64, true),
+ Field::new("gridHeight", DataType::UInt64, true),
+ Field::new("scaleX", DataType::Float64, true),
+ Field::new("scaleY", DataType::Float64, true),
+ Field::new("skewX", DataType::Float64, true),
+ Field::new("skewY", DataType::Float64, true),
+ Field::new("srid", DataType::Int32, true),
+ Field::new("numSampleDimensions", DataType::UInt32, true),
+ Field::new("tileWidth", DataType::UInt64, true),
+ Field::new("tileHeight", DataType::UInt64, true),
+ ])
+}
+
+#[derive(Debug)]
+struct RsMetaData {}
+
+impl SedonaScalarKernel for RsMetaData {
+ fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+ let matcher = ArgMatcher::new(
+ vec![ArgMatcher::is_raster()],
+ SedonaType::Arrow(DataType::Struct(metadata_struct_fields())),
+ );
+
+ matcher.match_args(args)
+ }
+
+ fn invoke_batch(
+ &self,
+ arg_types: &[SedonaType],
+ args: &[ColumnarValue],
+ ) -> Result<ColumnarValue> {
+ self.invoke_batch_from_args(arg_types, args, &SedonaType::Arrow(DataType::Null), 0, None)
+ }
+
+ fn invoke_batch_from_args(
+ &self,
+ arg_types: &[SedonaType],
+ args: &[ColumnarValue],
+ _return_type: &SedonaType,
+ _num_rows: usize,
+ config_options: Option<&ConfigOptions>,
+ ) -> Result<ColumnarValue> {
+ let executor = sedona_raster_functions::RasterExecutor::new(arg_types, args);
+ let capacity = executor.num_iterations();
+
+ let mut upper_left_x_builder = Float64Builder::with_capacity(capacity);
+ let mut upper_left_y_builder = Float64Builder::with_capacity(capacity);
+ let mut grid_width_builder = UInt64Builder::with_capacity(capacity);
+ let mut grid_height_builder = UInt64Builder::with_capacity(capacity);
+ let mut scale_x_builder = Float64Builder::with_capacity(capacity);
+ let mut scale_y_builder = Float64Builder::with_capacity(capacity);
+ let mut skew_x_builder = Float64Builder::with_capacity(capacity);
+ let mut skew_y_builder = Float64Builder::with_capacity(capacity);
+ let mut srid_builder = Int32Builder::with_capacity(capacity);
+ let mut num_bands_builder = UInt32Builder::with_capacity(capacity);
+ let mut tile_width_builder = UInt64Builder::with_capacity(capacity);
+ let mut tile_height_builder = UInt64Builder::with_capacity(capacity);
+
+ with_gdal(|gdal| {
+ configure_thread_local_options(gdal, config_options)?;
+ let provider = thread_local_provider(gdal)
+ .map_err(|e| exec_datafusion_err!("Failed to init GDAL provider: {e}"))?;
+
+ executor.execute_raster_void(|_i, raster_opt| {
+ match raster_opt {
+ None => {
+ upper_left_x_builder.append_null();
+ upper_left_y_builder.append_null();
+ grid_width_builder.append_null();
+ grid_height_builder.append_null();
+ scale_x_builder.append_null();
+ scale_y_builder.append_null();
+ skew_x_builder.append_null();
+ skew_y_builder.append_null();
+ srid_builder.append_null();
+ num_bands_builder.append_null();
+ tile_width_builder.append_null();
+ tile_height_builder.append_null();
+ }
+ Some(raster) => {
+ let metadata = raster.metadata();
+ let num_bands = raster.bands().len() as u32;
+
+ upper_left_x_builder.append_value(metadata.upper_left_x());
+ upper_left_y_builder.append_value(metadata.upper_left_y());
+ grid_width_builder.append_value(metadata.width());
+ grid_height_builder.append_value(metadata.height());
+ scale_x_builder.append_value(metadata.scale_x());
+ scale_y_builder.append_value(metadata.scale_y());
+ skew_x_builder.append_value(metadata.skew_x());
+ skew_y_builder.append_value(metadata.skew_y());
+
+ let srid = match raster.crs() {
+ None => 0i32,
+ Some(crs_str) => match deserialize_crs(crs_str) {
+ Ok(Some(crs_ref)) => {
+ crs_ref.srid().ok().flatten().map(|s| s as i32).unwrap_or(0)
+ }
Review Comment:
The SRID is parsed as a `u32` (`CoordinateReferenceSystem::srid() -> Option<u32>`), but this code casts with `as i32`. If a CRS reports an SRID > `i32::MAX`, this will wrap to a negative value. Prefer a checked conversion and fall back to `0` on overflow.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org