Skip to content

chore: Remove use of DataFusionError::Internal for sedonadb internal errors #564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paleolimbot merged 2 commits into from
Jan 30, 2026
Merged

chore: Remove use of DataFusionError::Internal for sedonadb internal errors #564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9498d4d
update errors
paleolimbot Jan 30, 2026
a703306
format
paleolimbot Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions c/sedona-proj/src/st_transform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use datafusion_common::cast::{as_string_view_array, as_struct_array};
use datafusion_common::config::ConfigOptions;
use datafusion_common::{exec_err, DataFusionError, Result, ScalarValue};
use datafusion_expr::ColumnarValue;
use sedona_common::sedona_internal_err;
use sedona_common::{sedona_internal_datafusion_err, sedona_internal_err};
use sedona_expr::item_crs::make_item_crs;
use sedona_expr::scalar_udf::{ScalarKernelRef, SedonaScalarKernel};
use sedona_functions::executor::WkbExecutor;
Expand Down Expand Up @@ -401,8 +401,8 @@ pub(crate) fn with_global_proj_engine(
// Otherwise, attempt to get the builder
let maybe_builder = PROJ_ENGINE_BUILDER.read().map_err(|_| {
// Highly unlikely (can only occur when a panic occurred during set)
DataFusionError::Internal(
"Failed to acquire read lock for global PROJ configuration".to_string(),
sedona_internal_datafusion_err!(
"Failed to acquire read lock for global PROJ configuration"
)
})?;

Expand All @@ -416,9 +416,7 @@ pub(crate) fn with_global_proj_engine(

engine_cell
.set(CachingCrsEngine::new(proj_engine))
.map_err(|_| {
DataFusionError::Internal("Failed to set cached PROJ transform".to_string())
})?;
.map_err(|_| sedona_internal_datafusion_err!("Failed to set cached PROJ transform"))?;
func(engine_cell.get().unwrap())?;
Ok(())
})
Expand Down
3 changes: 2 additions & 1 deletion rust/sedona-expr/src/statistics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use std::str::FromStr;

use datafusion_common::{stats::Precision, ColumnStatistics, DataFusionError, Result, ScalarValue};
use sedona_common::sedona_internal_datafusion_err;
use sedona_geometry::interval::{Interval, IntervalTrait};
use sedona_geometry::{
bounding_box::BoundingBox,
Expand Down Expand Up @@ -391,7 +392,7 @@ impl GeoStatistics {
pub fn to_scalar_value(&self) -> Result<ScalarValue> {
// Serialize to JSON
let serialized = serde_json::to_vec(self).map_err(|e| {
DataFusionError::Internal(format!("Failed to serialize GeoStatistics: {e}"))
sedona_internal_datafusion_err!("Failed to serialize GeoStatistics: {e}")
})?;

Ok(ScalarValue::Binary(Some(serialized)))
Expand Down
14 changes: 5 additions & 9 deletions rust/sedona-functions/src/st_analyze_agg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use datafusion_common::{
};
use datafusion_expr::{scalar_doc_sections::DOC_SECTION_OTHER, Documentation, Volatility};
use datafusion_expr::{Accumulator, ColumnarValue};
use sedona_common::{sedona_internal_datafusion_err, sedona_internal_err};
use sedona_expr::aggregate_udf::SedonaAccumulatorRef;
use sedona_expr::aggregate_udf::SedonaAggregateUDF;
use sedona_expr::item_crs::ItemCrsSedonaAccumulator;
Expand Down Expand Up @@ -383,9 +384,7 @@ impl AnalyzeAccumulator {
impl Accumulator for AnalyzeAccumulator {
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
if values.is_empty() {
return Err(DataFusionError::Internal(
"No input arrays provided to accumulator".to_string(),
));
return sedona_internal_err!("No input arrays provided to accumulator");
}
let arg_types = [self.input_type.clone()];
let arg_values = [ColumnarValue::Array(values[0].clone())];
Expand Down Expand Up @@ -441,9 +440,7 @@ impl Accumulator for AnalyzeAccumulator {
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> {
// Check input length (expecting 1 state field)
if states.is_empty() {
return Err(DataFusionError::Internal(
"No input arrays provided to accumulator in merge_batch".to_string(),
));
return sedona_internal_err!("No input arrays provided to accumulator in merge_batch");
}

let array = &states[0];
Expand All @@ -455,9 +452,8 @@ impl Accumulator for AnalyzeAccumulator {
}

let serialized = binary_array.value(i);
let other_stats: GeoStatistics = serde_json::from_slice(serialized).map_err(|e| {
DataFusionError::Internal(format!("Failed to deserialize stats: {e}"))
})?;
let other_stats: GeoStatistics = serde_json::from_slice(serialized)
.map_err(|e| sedona_internal_datafusion_err!("Failed to deserialize stats: {e}"))?;

// Use the merge method to combine statistics
self.stats.merge(&other_stats);
Expand Down
7 changes: 2 additions & 5 deletions rust/sedona-functions/src/st_envelope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::GeometryTrait;
use sedona_common::sedona_internal_err;
use sedona_expr::{
item_crs::ItemCrsKernel,
scalar_udf::{SedonaScalarKernel, SedonaScalarUDF},
Expand Down Expand Up @@ -125,11 +126,7 @@ fn invoke_scalar(wkb: &Wkb, writer: &mut impl std::io::Write) -> Result<()> {
geo_traits::GeometryType::GeometryCollection(_) => {
write_wkb_geometrycollection_header(writer, wkb.dim(), 0)
}
_ => {
return Err(DataFusionError::Internal(
"Unsupported geometry type".to_string(),
))
}
_ => return sedona_internal_err!("Unsupported geometry type"),
};

if let Err(e) = result {
Expand Down
4 changes: 1 addition & 3 deletions rust/sedona-functions/src/st_envelope_agg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ impl BoundsAccumulator2D {
// Check the input length for update methods.
fn check_update_input_len(input: &[ArrayRef], expected: usize, context: &str) -> Result<()> {
if input.is_empty() {
return Err(DataFusionError::Internal(format!(
"No input arrays provided to accumulator in {context}"
)));
return sedona_internal_err!("No input arrays provided to accumulator in {context}");
}
if input.len() != expected {
return sedona_internal_err!(
Expand Down
10 changes: 6 additions & 4 deletions rust/sedona-functions/src/st_geomfromwkt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ use std::{str::FromStr, sync::Arc, vec};
use arrow_array::builder::{BinaryBuilder, StringViewBuilder};
use arrow_schema::DataType;
use datafusion_common::cast::as_string_view_array;
use datafusion_common::error::{DataFusionError, Result};
use datafusion_common::error::Result;
use datafusion_common::exec_datafusion_err;
use datafusion_common::scalar::ScalarValue;
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use sedona_common::sedona_internal_datafusion_err;
use sedona_expr::item_crs::make_item_crs;
use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES;
Expand Down Expand Up @@ -136,8 +138,8 @@ impl SedonaScalarKernel for STGeoFromWKT {
}

fn invoke_scalar(wkt_bytes: &str, builder: &mut BinaryBuilder) -> Result<()> {
let geometry: Wkt<f64> = Wkt::from_str(wkt_bytes)
.map_err(|err| DataFusionError::Internal(format!("WKT parse error: {err}")))?;
let geometry: Wkt<f64> =
Wkt::from_str(wkt_bytes).map_err(|err| exec_datafusion_err!("WKT parse error: {err}"))?;

write_geometry(
builder,
Expand All @@ -146,7 +148,7 @@ fn invoke_scalar(wkt_bytes: &str, builder: &mut BinaryBuilder) -> Result<()> {
endianness: Endianness::LittleEndian,
},
)
.map_err(|err| DataFusionError::Internal(format!("WKB write error: {err}")))
.map_err(|err| sedona_internal_datafusion_err!("WKB write error: {err}"))
}

/// ST_GeomFromEWKT() UDF implementation
Expand Down
9 changes: 3 additions & 6 deletions rust/sedona-functions/src/st_pointzm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::Dimensions;
use sedona_common::{sedona_internal_datafusion_err, sedona_internal_err};
use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
use sedona_geometry::{
error::SedonaGeometryError,
Expand Down Expand Up @@ -166,9 +167,7 @@ impl SedonaScalarKernel for STGeoFromPointZm {
.iter()
.map(|v| match v {
ColumnarValue::Scalar(ScalarValue::Float64(val)) => Ok(*val),
_ => Err(datafusion_common::DataFusionError::Internal(
"Expected Float64 scalar".to_string(),
)),
_ => sedona_internal_err!("Expected Float64 scalar"),
})
.collect();
let scalar_coords = scalar_coords?;
Expand Down Expand Up @@ -216,9 +215,7 @@ impl SedonaScalarKernel for STGeoFromPointZm {
let values = arrays.iter().map(|v| v.value(i)).collect::<Vec<_>>();
if !any_null {
write_wkb_pointzm(&mut builder, &values, self.dim).map_err(|_| {
datafusion_common::DataFusionError::Internal(
"Failed to write WKB point header".to_string(),
)
sedona_internal_datafusion_err!("Failed to write WKB point header")
})?;
builder.append_value([]);
} else {
Expand Down
6 changes: 3 additions & 3 deletions rust/sedona-functions/src/st_setsrid.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,9 @@ mod test {
to: ScalarValue,
) -> Result<(SedonaType, ColumnarValue)> {
let SedonaType::Arrow(datatype) = &arg_type[1] else {
return Err(DataFusionError::Internal(
"Expected SedonaType::Arrow, but found a different variant".to_string(),
));
return sedona_internal_err!(
"Expected SedonaType::Arrow, but found a different variant"
);
};
let arg_fields = vec![
Arc::new(arg_type[0].to_storage_field("", true)?),
Expand Down
4 changes: 2 additions & 2 deletions rust/sedona-functions/src/st_xyzm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use geo_traits::{
CoordTrait, Dimensions, GeometryCollectionTrait, GeometryTrait, LineStringTrait,
MultiLineStringTrait, MultiPointTrait, MultiPolygonTrait, PointTrait, PolygonTrait,
};
use sedona_common::sedona_internal_err;
use sedona_common::{sedona_internal_datafusion_err, sedona_internal_err};
use sedona_expr::{
item_crs::ItemCrsKernel,
scalar_udf::{SedonaScalarKernel, SedonaScalarUDF},
Expand Down Expand Up @@ -173,7 +173,7 @@ fn invoke_scalar(item: &Wkb, dim_index: usize) -> Result<Option<f64>> {
1 => {
let coord_dim = multipoint.dim();
let point = MultiPointTrait::point(multipoint, 0)
.ok_or(DataFusionError::Internal("Missing point".to_string()))?;
.ok_or(sedona_internal_datafusion_err!("Missing point"))?;
let coord = PointTrait::coord(&point);
return get_coord(coord_dim, coord, dim_index);
}
Expand Down
17 changes: 8 additions & 9 deletions rust/sedona-functions/src/st_xyzm_minmax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ use std::sync::Arc;
use crate::executor::WkbExecutor;
use arrow_array::builder::Float64Builder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, DataFusionError};
use datafusion_common::error::Result;
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::GeometryTrait;
use sedona_common::sedona_internal_err;
use sedona_common::{sedona_internal_datafusion_err, sedona_internal_err};
use sedona_expr::{
item_crs::ItemCrsKernel,
scalar_udf::{SedonaScalarKernel, SedonaScalarUDF},
Expand Down Expand Up @@ -196,24 +196,23 @@ fn invoke_scalar(
let interval: Interval = match dim {
"x" => {
let xy_bounds = geo_traits_bounds_xy(item)
.map_err(|e| DataFusionError::Internal(format!("Error updating bounds: {e}")))?;
Interval::try_from(*xy_bounds.x()).map_err(|e| {
DataFusionError::Internal(format!("Error converting to interval: {e}"))
})?
.map_err(|e| sedona_internal_datafusion_err!("Error updating bounds: {e}"))?;
Interval::try_from(*xy_bounds.x())
.map_err(|e| sedona_internal_datafusion_err!("Error converting to interval: {e}"))?
}
"y" => {
let xy_bounds = geo_traits_bounds_xy(item)
.map_err(|e| DataFusionError::Internal(format!("Error updating bounds: {e}")))?;
.map_err(|e| sedona_internal_datafusion_err!("Error updating bounds: {e}"))?;
*xy_bounds.y()
}
"z" => {
let z_bounds = geo_traits_bounds_z(item)
.map_err(|e| DataFusionError::Internal(format!("Error updating bounds: {e}")))?;
.map_err(|e| sedona_internal_datafusion_err!("Error updating bounds: {e}"))?;
z_bounds
}
"m" => {
let m_bounds = geo_traits_bounds_m(item)
.map_err(|e| DataFusionError::Internal(format!("Error updating bounds: {e}")))?;
.map_err(|e| sedona_internal_datafusion_err!("Error updating bounds: {e}"))?;
m_bounds
}
_ => sedona_internal_err!("unexpected dim index")?,
Expand Down
6 changes: 2 additions & 4 deletions rust/sedona-geo/src/centroid.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// under the License.
//! Centroid extraction functionality for WKB geometries

use datafusion_common::{error::DataFusionError, Result};
use datafusion_common::{exec_err, Result};
use geo_traits::CoordTrait;
use geo_traits::GeometryTrait;
use geo_traits::PointTrait;
Expand Down Expand Up @@ -56,9 +56,7 @@ pub fn extract_centroid_2d(geo: impl GeometryTrait<T = f64>) -> Result<(f64, f64
// Return POINT EMPTY as (NaN, NaN)
Ok((f64::NAN, f64::NAN))
} else {
Err(DataFusionError::Internal(
"Centroid computation failed.".to_string(),
))
exec_err!("Centroid computation failed.")
}
}
}
Expand Down
18 changes: 5 additions & 13 deletions rust/sedona-geo/src/st_intersection_agg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ use std::{sync::Arc, vec};

use arrow_array::ArrayRef;
use arrow_schema::FieldRef;
use datafusion_common::{
error::{DataFusionError, Result},
ScalarValue,
};
use datafusion_common::{error::Result, exec_err, ScalarValue};
use datafusion_expr::{Accumulator, ColumnarValue};
use geo::{BooleanOps, Intersects};
use geo_traits::to_geo::ToGeoGeometry;
use sedona_common::sedona_internal_err;
use sedona_expr::{
aggregate_udf::{SedonaAccumulator, SedonaAccumulatorRef},
item_crs::ItemCrsSedonaAccumulator,
Expand Down Expand Up @@ -104,9 +102,7 @@ impl IntersectionAccumulator {
geo::Geometry::Polygon(poly) => geo::MultiPolygon(vec![poly]),
geo::Geometry::MultiPolygon(multi) => multi.clone(),
_ => {
return Err(DataFusionError::Internal(
"Unsupported geometry type for intersection operation".to_string(),
));
return exec_err!("Unsupported geometry type for intersection operation");
}
};

Expand Down Expand Up @@ -170,9 +166,7 @@ impl IntersectionAccumulator {
impl Accumulator for IntersectionAccumulator {
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
if values.is_empty() {
return Err(DataFusionError::Internal(
"No input arrays provided to accumulator in update_batch".to_string(),
));
return sedona_internal_err!("No input arrays provided to accumulator in update_batch");
}
let arg_types = [self.input_type.clone()];
let args = [ColumnarValue::Array(values[0].clone())];
Expand Down Expand Up @@ -213,9 +207,7 @@ impl Accumulator for IntersectionAccumulator {
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> {
// Check input length (expecting 1 state field)
if states.is_empty() {
return Err(DataFusionError::Internal(
"No input arrays provided to accumulator in merge_batch".to_string(),
));
return sedona_internal_err!("No input arrays provided to accumulator in merge_batch");
}
let array = &states[0];
let args = [ColumnarValue::Array(array.clone())];
Expand Down
13 changes: 3 additions & 10 deletions rust/sedona-geo/src/st_union_agg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ use std::{sync::Arc, vec};

use arrow_array::ArrayRef;
use arrow_schema::FieldRef;
use datafusion_common::{
error::{DataFusionError, Result},
ScalarValue,
};
use datafusion_common::{error::Result, exec_err, ScalarValue};
use datafusion_expr::{Accumulator, ColumnarValue};
use geo::BooleanOps;
use geo_traits::to_geo::ToGeoGeometry;
Expand Down Expand Up @@ -100,9 +97,7 @@ impl UnionAccumulator {
geo::Geometry::Polygon(poly) => geo::MultiPolygon(vec![poly]),
geo::Geometry::MultiPolygon(multi) => multi.clone(),
_ => {
return Err(DataFusionError::Internal(
"Unsupported geometry type for union operation".to_string(),
));
return exec_err!("Unsupported geometry type for union operation");
}
};

Expand Down Expand Up @@ -206,9 +201,7 @@ impl Accumulator for UnionAccumulator {
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> {
// Check input length (expecting 1 state field)
if states.is_empty() {
return Err(DataFusionError::Internal(
"No input arrays provided to accumulator in merge_batch".to_string(),
));
return sedona_internal_err!("No input arrays provided to accumulator in merge_batch");
}
let array = &states[0];
let args = [ColumnarValue::Array(array.clone())];
Expand Down
Loading