Skip to content

Commit a715173

Browse files
crepererumcompheadalamb
authored
refactor: shrink SchemaError (#16653)
One step towards #16652. Co-authored-by: Oleks V <comphead@users.noreply.github.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
1 parent 0185da6 commit a715173

File tree

4 files changed

+24
-18
lines changed

4 files changed

+24
-18
lines changed

datafusion/common/src/column.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ impl Column {
262262

263263
// If not due to USING columns then due to ambiguous column name
264264
return _schema_err!(SchemaError::AmbiguousReference {
265-
field: Column::new_unqualified(&self.name),
265+
field: Box::new(Column::new_unqualified(&self.name)),
266266
})
267267
.map_err(|err| {
268268
let mut diagnostic = Diagnostic::new_error(

datafusion/common/src/dfschema.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ impl DFSchema {
229229
for (qualifier, name) in qualified_names {
230230
if unqualified_names.contains(name) {
231231
return _schema_err!(SchemaError::AmbiguousReference {
232-
field: Column::new(Some(qualifier.clone()), name)
232+
field: Box::new(Column::new(Some(qualifier.clone()), name))
233233
});
234234
}
235235
}
@@ -489,7 +489,7 @@ impl DFSchema {
489489
Ok((fields_without_qualifier[0].0, fields_without_qualifier[0].1))
490490
} else {
491491
_schema_err!(SchemaError::AmbiguousReference {
492-
field: Column::new_unqualified(name.to_string(),),
492+
field: Box::new(Column::new_unqualified(name.to_string()))
493493
})
494494
}
495495
}

datafusion/common/src/error.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ macro_rules! context {
164164
#[derive(Debug)]
165165
pub enum SchemaError {
166166
/// Schema contains a (possibly) qualified and unqualified field with same unqualified name
167-
AmbiguousReference { field: Column },
167+
AmbiguousReference { field: Box<Column> },
168168
/// Schema contains duplicate qualified field name
169169
DuplicateQualifiedField {
170170
qualifier: Box<TableReference>,
@@ -951,14 +951,18 @@ pub fn add_possible_columns_to_diag(
951951

952952
#[cfg(test)]
953953
mod test {
954+
use super::*;
955+
954956
use std::mem::size_of;
955957
use std::sync::Arc;
956958

957-
use crate::error::{DataFusionError, GenericError};
958959
use arrow::error::ArrowError;
959960

960961
#[test]
961-
fn test_datafusion_error_size() {
962+
fn test_error_size() {
963+
// Since Errors influence the size of Result which influence the size of the stack
964+
// please don't allow this to grow larger
965+
assert_eq!(size_of::<SchemaError>(), 40);
962966
assert_eq!(size_of::<DataFusionError>(), 40);
963967
}
964968

@@ -1128,7 +1132,7 @@ mod test {
11281132
);
11291133

11301134
// assert wrapping other Error
1131-
let generic_error: GenericError = Box::new(std::io::Error::other("io error"));
1135+
let generic_error: GenericError = Box::new(io::Error::other("io error"));
11321136
let datafusion_error: DataFusionError = generic_error.into();
11331137
println!("{}", datafusion_error.strip_backtrace());
11341138
assert_eq!(
@@ -1139,7 +1143,7 @@ mod test {
11391143

11401144
#[test]
11411145
fn external_error_no_recursive() {
1142-
let generic_error_1: GenericError = Box::new(std::io::Error::other("io error"));
1146+
let generic_error_1: GenericError = Box::new(io::Error::other("io error"));
11431147
let external_error_1: DataFusionError = generic_error_1.into();
11441148
let generic_error_2: GenericError = Box::new(external_error_1);
11451149
let external_error_2: DataFusionError = generic_error_2.into();
@@ -1159,7 +1163,7 @@ mod test {
11591163

11601164
/// Model what happens when using arrow kernels in DataFusion
11611165
/// code: need to turn an ArrowError into a DataFusionError
1162-
fn return_datafusion_error() -> crate::error::Result<()> {
1166+
fn return_datafusion_error() -> Result<()> {
11631167
// Expect the '?' to work
11641168
Err(ArrowError::SchemaError("bar".to_string()).into())
11651169
}

datafusion/expr/src/logical_plan/builder.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2535,15 +2535,17 @@ mod tests {
25352535

25362536
match plan {
25372537
Err(DataFusionError::SchemaError(err, _)) => {
2538-
if let SchemaError::AmbiguousReference {
2539-
field:
2540-
Column {
2541-
relation: Some(TableReference::Bare { table }),
2542-
name,
2543-
..
2544-
},
2545-
} = *err
2546-
{
2538+
if let SchemaError::AmbiguousReference { field } = *err {
2539+
let Column {
2540+
relation,
2541+
name,
2542+
spans: _,
2543+
} = *field;
2544+
let Some(TableReference::Bare { table }) = relation else {
2545+
return plan_err!(
2546+
"wrong relation: {relation:?}, expected table name"
2547+
);
2548+
};
25472549
assert_eq!(*"employee_csv", *table);
25482550
assert_eq!("id", &name);
25492551
Ok(())

0 commit comments

Comments
 (0)