Skip to content

Commit

Permalink
More progress
Browse files Browse the repository at this point in the history
  • Loading branch information
wjones127 committed Feb 10, 2025
1 parent 3f2ef07 commit 0e101dc
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 45 deletions.
84 changes: 45 additions & 39 deletions rust/lance-core/src/datatypes/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use arrow_array::{
};
use arrow_schema::{DataType, Field as ArrowField};
use deepsize::DeepSizeOf;
use http::StatusCode;

Check warning on line 23 in rust/lance-core/src/datatypes/field.rs

View workflow job for this annotation

GitHub Actions / linux-build (nightly)

unused import: `http::StatusCode`

Check warning on line 23 in rust/lance-core/src/datatypes/field.rs

View workflow job for this annotation

GitHub Actions / linux-arm

unused import: `http::StatusCode`
use lance_arrow::{bfloat16::ARROW_EXT_NAME_KEY, *};
use snafu::{location, Location};

use super::{
schema::{compare_fields, explain_fields_difference},
Expand Down Expand Up @@ -100,10 +100,10 @@ impl FromStr for StorageClass {
match s {
"default" | "" => Ok(Self::Default),
"blob" => Ok(Self::Blob),
_ => Err(Error::Schema {
message: format!("Unknown storage class: {}", s),
location: location!(),
}),
_ => Err(Error::bad_request(
"Unknown storage class",
format!("Received: {}. Expected 'default' or 'blob'.", s),
)),
}
}
}
Expand Down Expand Up @@ -562,16 +562,16 @@ impl Field {
}

/// Project by a field.
///
pub fn project_by_field(&self, other: &Self, on_type_mismatch: OnTypeMismatch) -> Result<Self> {
if self.name != other.name {
return Err(Error::Schema {
message: format!(
return Err(Error::internal(
"Bad projection",
format!(
"Attempt to project field by different names: {} and {}",
self.name, other.name,
),
location: location!(),
});
)
.with_backtrace());
};

match (self.data_type(), other.data_type()) {
Expand All @@ -581,27 +581,29 @@ impl Field {
|| (dt.is_binary_like() && other_dt.is_binary_like()) =>
{
if dt != other_dt {
return Err(Error::Schema {
message: format!(
return Err(Error::internal(
"Bad projection",
format!(
"Attempt to project field by different types: {} and {}",
dt, other_dt,
),
location: location!(),
});
)
.with_backtrace());
}
Ok(self.clone())
}
(DataType::Struct(_), DataType::Struct(_)) => {
let mut fields = vec![];
for other_field in other.children.iter() {
let Some(child) = self.child(&other_field.name) else {
return Err(Error::Schema {
message: format!(
return Err(Error::internal(
"Bad projection",
format!(
"Attempt to project non-existed field: {} on {}",
other_field.name, self,
),
location: location!(),
});
)
.with_backtrace());
};
fields.push(child.project_by_field(other_field, on_type_mismatch)?);
}
Expand Down Expand Up @@ -633,13 +635,14 @@ impl Field {
Ok(self.clone())
}
_ => match on_type_mismatch {
OnTypeMismatch::Error => Err(Error::Schema {
message: format!(
"Attempt to project incompatible fields: {} and {}",
self, other
OnTypeMismatch::Error => Err(Error::internal(
"Bad projection",
format!(
"Attempt to project field by different types: {} and {}",
self, other,
),
location: location!(),
}),
)
.with_backtrace()),
OnTypeMismatch::TakeSelf => Ok(self.clone()),
},
}
Expand All @@ -664,13 +667,14 @@ impl Field {

pub(crate) fn do_intersection(&self, other: &Self, ignore_types: bool) -> Result<Self> {
if self.name != other.name {
return Err(Error::Arrow {
message: format!(
return Err(Error::internal(
"Bad intersection",
format!(
"Attempt to intersect different fields: {} and {}",
self.name, other.name,
),
location: location!(),
});
)
.with_backtrace());
}
let self_type = self.data_type();
let other_type = other.data_type();
Expand Down Expand Up @@ -703,13 +707,14 @@ impl Field {
}

if (!ignore_types && self_type != other_type) || self.name != other.name {
return Err(Error::Arrow {
message: format!(
"Attempt to intersect different fields: ({}, {}) and ({}, {})",
self.name, self_type, other.name, other_type
return Err(Error::internal(
"Bad intersection",
format!(
"Attempt to intersect different fields: {} and {}",
self, other,
),
location: location!(),
});
)
.with_backtrace());
}

Ok(if self.id >= 0 {
Expand Down Expand Up @@ -794,13 +799,14 @@ impl Field {
}
_ => {
if self.data_type() != other.data_type() {
return Err(Error::Schema {
message: format!(
return Err(Error::internal(
"Bad merge",
format!(
"Attempt to merge incompatible fields: {} and {}",
self, other
self, other,
),
location: location!(),
});
)
.with_backtrace());
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions rust/lance-core/src/datatypes/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use arrow_array::RecordBatch;
use arrow_schema::{Field as ArrowField, Schema as ArrowSchema};
use deepsize::DeepSizeOf;
use lance_arrow::*;
use snafu::{location, Location};

use super::field::{Field, OnTypeMismatch, SchemaCompareOptions, StorageClass};
use crate::{Error, Result, ROW_ADDR, ROW_ID};
Expand Down Expand Up @@ -136,11 +135,11 @@ impl Schema {
pub fn check_compatible(&self, expected: &Self, options: &SchemaCompareOptions) -> Result<()> {
if !self.compare_with_options(expected, options) {
let difference = self.explain_difference(expected, options);
Err(Error::SchemaMismatch {
// unknown reason is messy but this shouldn't happen.
difference: difference.unwrap_or("unknown reason".to_string()),
location: location!(),
})
Err(Error::bad_request(
"Schema mismatch",
difference.unwrap_or("unknown reason".to_string()),
)
.with_backtrace())
} else {
Ok(())
}
Expand Down
9 changes: 9 additions & 0 deletions rust/lance-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ impl Error {
Self::new(StatusCode::BAD_REQUEST, title, details)
}

pub fn internal(title: &'static str, details: impl Into<String>) -> Self {
Self::new(StatusCode::INTERNAL_SERVER_ERROR, title, details)
}

pub fn unexpected(title: &'static str, details: impl Into<String>) -> Self {
Self::new(StatusCode::INTERNAL_SERVER_ERROR, title, details)
}
Expand All @@ -40,6 +44,11 @@ impl Error {
self
}

/// Capture a backtrace for this error.
///
/// Backtraces aren't captured by default because they can be expensive to
/// capture. They are most helpful in functions that can be called from
/// many different code paths. For example, low-level IO functions.
pub fn with_backtrace(mut self) -> Self {
self.0.backtrace = MaybeBacktrace::Captured(Backtrace::capture());
self
Expand Down

0 comments on commit 0e101dc

Please sign in to comment.