Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions Cargo.lock

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

34 changes: 34 additions & 0 deletions rust/sedona-common/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// Macro to create Sedona Internal Error that avoids the misleading error message from
/// DataFusionError::Internal.
#[macro_export]
macro_rules! sedona_internal_err {
($($args:expr),*) => {{
let msg = std::format!(
"SedonaDB internal error: {}{}.\nThis issue was likely caused by a bug in SedonaDB's code. \
Please help us to resolve this by filing a bug report in our issue tracker: \
https://github.com/apache/sedona-db/issues",
std::format!($($args),*),
datafusion_common::DataFusionError::get_back_trace(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get the same linking errors in CI that we got before, you could try removing the back trace feature (we can always add it back in later if it turns out that we're having trouble debugging user errors)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's what you meant

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it passed !

);
// We avoid using Internal to avoid the message suggesting it's internal to DataFusion
Err(datafusion_common::DataFusionError::External(msg.into()))
}};
}

#[cfg(test)]
mod tests {
use datafusion_common::DataFusionError;

#[test]
fn test_sedona_internal_err() {
let result: Result<(), DataFusionError> = sedona_internal_err!("Test error: {}", "details");
assert!(result.is_err());
let err = result.unwrap_err();
let err_string = err.to_string();
assert!(err_string.contains("SedonaDB internal error: Test error: details"));

// Ensure the message doesn't contain the 'DataFusion'-specific messages
assert!(!err_string.contains("DataFusion's code"));
assert!(!err_string.contains("https://github.com/apache/datafusion/issues"));
}
}
1 change: 1 addition & 0 deletions rust/sedona-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! This crate contains shared components that are used across multiple
//! Sedona modules, including configuration options and common data structures.

pub mod error;
pub mod option;

pub use option::*;
1 change: 1 addition & 0 deletions rust/sedona-expr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ datafusion-common = { workspace = true }
datafusion-expr = { workspace = true }
datafusion-physical-expr = { workspace = true }
geo-traits = { workspace = true }
sedona-common = { path = "../sedona-common" }
sedona-geometry = { path = "../sedona-geometry" }
sedona-schema = { path = "../sedona-schema" }
serde = { workspace = true }
Expand Down
10 changes: 5 additions & 5 deletions rust/sedona-expr/src/function_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use crate::{
aggregate_udf::{SedonaAccumulatorRef, SedonaAggregateUDF},
scalar_udf::{ScalarKernelRef, SedonaScalarUDF},
};
use datafusion_common::{error::Result, internal_err};
use datafusion_common::error::Result;
use datafusion_expr::{AggregateUDFImpl, ScalarUDFImpl};
use sedona_common::sedona_internal_err;
use std::collections::HashMap;

/// Helper for managing groups of functions
Expand Down Expand Up @@ -110,7 +111,7 @@ impl FunctionSet {
function.add_kernel(kernel);
Ok(self.aggregate_udf(name).unwrap())
} else {
internal_err!("Can't register aggregate kernel for function '{}'", name)
sedona_internal_err!("Can't register aggregate kernel for function '{}'", name)
}
}
}
Expand Down Expand Up @@ -248,10 +249,9 @@ mod tests {
let err = functions
.add_aggregate_udf_kernel("function that does not exist", kernel.clone())
.unwrap_err();
assert_eq!(
err.message().lines().next().unwrap(),
assert!(err.message().lines().next().unwrap().contains(
"Can't register aggregate kernel for function 'function that does not exist'."
);
));

let udaf2 = SedonaAggregateUDF::new(
"simple_udaf2",
Expand Down
9 changes: 5 additions & 4 deletions rust/sedona-expr/src/scalar_udf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use std::sync::Arc;
use std::{any::Any, fmt::Debug};

use arrow_schema::{DataType, Field, FieldRef};
use datafusion_common::{internal_err, not_impl_err, plan_err, Result, ScalarValue};
use datafusion_common::{not_impl_err, plan_err, Result, ScalarValue};
use datafusion_expr::{
ColumnarValue, Documentation, ReturnFieldArgs, ScalarFunctionArgs, ScalarUDFImpl, Signature,
Volatility,
};
use sedona_common::sedona_internal_err;
use sedona_schema::datatypes::{Edges, SedonaType};

pub type ScalarKernelRef = Arc<dyn SedonaScalarKernel + Send + Sync>;
Expand Down Expand Up @@ -121,7 +122,7 @@ impl ArgMatcher {
(Some(lhs_crs), Some(rhs_crs)) => {
plan_err!("Mismatched CRS arguments: {lhs_crs} vs {rhs_crs}\n{hint}")
}
_ => internal_err!("None vs. None should be considered equal"),
_ => sedona_internal_err!("None vs. None should be considered equal"),
};
}
}
Expand Down Expand Up @@ -876,13 +877,13 @@ mod tests {
}
}

internal_err!("unrecognized target value")
sedona_internal_err!("unrecognized target value")
}
}

impl SedonaScalarKernel for SimpleCast {
fn return_type(&self, _args: &[SedonaType]) -> Result<Option<SedonaType>> {
internal_err!("Should not be called")
sedona_internal_err!("Should not be called")
}

fn return_type_from_args_and_scalars(
Expand Down
13 changes: 9 additions & 4 deletions rust/sedona-expr/src/spatial_filter.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use std::sync::Arc;

use datafusion_common::{internal_err, DataFusionError, Result, ScalarValue};
use datafusion_common::{DataFusionError, Result, ScalarValue};
use datafusion_expr::Operator;
use datafusion_physical_expr::{
expressions::{BinaryExpr, Column, Literal},
PhysicalExpr, ScalarFunctionExpr,
};
use geo_traits::Dimensions;
use sedona_common::sedona_internal_err;
use sedona_geometry::{bounding_box::BoundingBox, bounds::wkb_bounds_xy, interval::IntervalTrait};
use sedona_schema::datatypes::SedonaType;

Expand Down Expand Up @@ -107,7 +108,9 @@ impl SpatialFilter {
match scalar_fun.fun().name() {
"st_intersects" => {
if args.len() != 2 {
return internal_err!("unexpected argument count in filter evaluation");
return sedona_internal_err!(
"unexpected argument count in filter evaluation"
);
}

match (&args[0], &args[1]) {
Expand All @@ -126,7 +129,9 @@ impl SpatialFilter {
}
"st_hasz" => {
if args.len() != 1 {
return internal_err!("unexpected argument count in filter evaluation");
return sedona_internal_err!(
"unexpected argument count in filter evaluation"
);
}

match &args[0] {
Expand Down Expand Up @@ -191,7 +196,7 @@ fn literal_bounds(literal: &Literal) -> Result<BoundingBox> {
_ => {}
}

internal_err!("Unexpected scalar type in filter expression")
sedona_internal_err!("Unexpected scalar type in filter expression")
}

fn parse_args(args: &[Arc<dyn PhysicalExpr>]) -> Vec<ArgRef<'_>> {
Expand Down
1 change: 1 addition & 0 deletions rust/sedona-functions/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ arrow-array = { workspace = true }
datafusion-common = { workspace = true }
datafusion-expr = { workspace = true }
geo-traits = { workspace = true }
sedona-common = { path = "../sedona-common" }
sedona-expr = { path = "../sedona-expr" }
sedona-geometry = { path = "../sedona-geometry" }
sedona-schema = { path = "../sedona-schema" }
Expand Down
11 changes: 6 additions & 5 deletions rust/sedona-functions/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ use std::iter::zip;
use arrow_array::ArrayRef;
use datafusion_common::cast::{as_binary_array, as_binary_view_array};
use datafusion_common::error::Result;
use datafusion_common::{internal_err, DataFusionError, ScalarValue};
use datafusion_common::{DataFusionError, ScalarValue};
use datafusion_expr::ColumnarValue;
use sedona_common::sedona_internal_err;
use sedona_schema::datatypes::SedonaType;
use wkb::reader::Wkb;

Expand Down Expand Up @@ -300,7 +301,7 @@ impl IterGeo for ArrayRef {
func: F,
) -> Result<()> {
if num_iterations != self.len() {
return internal_err!(
return sedona_internal_err!(
"Expected {num_iterations} items but got Array with {} items",
self.len()
);
Expand All @@ -313,7 +314,7 @@ impl IterGeo for ArrayRef {
// We could cast here as a fallback, iterate and cast per-element, or
// implement iter_as_something_else()/supports_iter_xxx() when more geo array types
// are supported.
internal_err!("Can't iterate over {:?} as Wkb", sedona_type)
sedona_internal_err!("Can't iterate over {:?} as Wkb", sedona_type)
}
}
}
Expand All @@ -326,7 +327,7 @@ impl ScalarGeo for ScalarValue {
| ScalarValue::BinaryView(maybe_item)
| ScalarValue::LargeBinary(maybe_item) => Ok(maybe_item.as_deref()),
ScalarValue::Null => Ok(None),
_ => internal_err!("Can't iterate over {:?} ScalarValue as &[u8]", self),
_ => sedona_internal_err!("Can't iterate over {:?} ScalarValue as &[u8]", self),
}
}
}
Expand Down Expand Up @@ -379,7 +380,7 @@ fn iter_wkb_wkb_array<
_ => {
// We could do casting of one or both sides to support other cases as they
// arise to manage the complexity/performance balance
internal_err!(
sedona_internal_err!(
"Can't iterate over {:?} and {:?} arrays as a pair of Wkb scalars",
types.0,
types.1
Expand Down
5 changes: 3 additions & 2 deletions rust/sedona-functions/src/st_dimension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use std::sync::Arc;
use crate::executor::WkbExecutor;
use arrow_array::builder::Int8Builder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, internal_err};
use datafusion_common::error::Result;
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::{GeometryCollectionTrait, GeometryTrait, GeometryType};
use sedona_common::sedona_internal_err;
use sedona_expr::scalar_udf::{ArgMatcher, SedonaScalarKernel, SedonaScalarUDF};
use sedona_schema::datatypes::SedonaType;
use wkb::reader::Wkb;
Expand Down Expand Up @@ -76,7 +77,7 @@ fn invoke_scalar(item: &Wkb) -> Result<i8> {
}
Ok(highest_dim)
}
_ => internal_err!("Invalid geometry type"),
_ => sedona_internal_err!("Invalid geometry type"),
}
}

Expand Down
5 changes: 3 additions & 2 deletions rust/sedona-functions/src/st_envelope_aggr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ use arrow_array::ArrayRef;
use arrow_schema::FieldRef;
use datafusion_common::{
error::{DataFusionError, Result},
internal_err, ScalarValue,
ScalarValue,
};
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, Accumulator, ColumnarValue, Documentation, Volatility,
};
use sedona_common::sedona_internal_err;
use sedona_expr::{
aggregate_udf::{SedonaAccumulator, SedonaAggregateUDF},
scalar_udf::ArgMatcher,
Expand Down Expand Up @@ -108,7 +109,7 @@ impl BoundsAccumulator2D {
)));
}
if input.len() != expected {
return internal_err!(
return sedona_internal_err!(
"Unexpected input length in {} (expected {}, got {})",
context,
expected,
Expand Down
5 changes: 3 additions & 2 deletions rust/sedona-functions/src/st_geometrytype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use std::sync::Arc;
use crate::executor::WkbExecutor;
use arrow_array::builder::StringBuilder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, internal_err};
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_expr::scalar_udf::{ArgMatcher, SedonaScalarKernel, SedonaScalarUDF};
use sedona_schema::datatypes::SedonaType;
use wkb::reader::Wkb;
Expand Down Expand Up @@ -80,7 +81,7 @@ fn invoke_scalar(item: &Wkb) -> Result<Option<String>> {
}

// Other geometry types in geo that we should not get here: Rect, Triangle, Line
_ => internal_err!("unexpected geometry type"),
_ => sedona_internal_err!("unexpected geometry type"),
}
}

Expand Down
5 changes: 3 additions & 2 deletions rust/sedona-functions/src/st_haszm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use std::sync::Arc;
use crate::executor::WkbExecutor;
use arrow_array::builder::BooleanBuilder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, internal_err};
use datafusion_common::error::Result;
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::{Dimensions, GeometryTrait};
use sedona_common::sedona_internal_err;
use sedona_expr::scalar_udf::{ArgMatcher, SedonaScalarKernel, SedonaScalarUDF};
use sedona_schema::datatypes::SedonaType;
use wkb::reader::Wkb;
Expand Down Expand Up @@ -108,7 +109,7 @@ fn invoke_scalar(item: &Wkb, dim_index: usize) -> Result<Option<bool>> {
match dim_index {
2 => Ok(Some(matches!(geom_dim, Dimensions::Xyz | Dimensions::Xyzm))),
3 => Ok(Some(matches!(geom_dim, Dimensions::Xym | Dimensions::Xyzm))),
_ => internal_err!("unexpected dim_index"),
_ => sedona_internal_err!("unexpected dim_index"),
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions rust/sedona-functions/src/st_isempty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ use std::sync::Arc;
use crate::executor::WkbExecutor;
use arrow_array::builder::BooleanBuilder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, internal_err};
use datafusion_common::error::Result;
use datafusion_expr::{
scalar_doc_sections::DOC_SECTION_OTHER, ColumnarValue, Documentation, Volatility,
};
use geo_traits::{
GeometryCollectionTrait, GeometryTrait, LineStringTrait, MultiLineStringTrait, MultiPointTrait,
MultiPolygonTrait, PointTrait, PolygonTrait,
};
use sedona_common::sedona_internal_err;
use sedona_expr::scalar_udf::{ArgMatcher, SedonaScalarKernel, SedonaScalarUDF};
use sedona_schema::datatypes::SedonaType;
use wkb::reader::Wkb;
Expand Down Expand Up @@ -87,7 +88,7 @@ fn invoke_scalar(item: &Wkb) -> Result<bool> {
geo_traits::GeometryType::GeometryCollection(geometrycollection) => {
Ok(geometrycollection.num_geometries() == 0)
}
_ => internal_err!("Invalid geometry type"),
_ => sedona_internal_err!("Invalid geometry type"),
}
}

Expand Down
Loading