Skip to content
This repository was archived by the owner on Sep 12, 2018. It is now read-only.

Make sure mentat error types preserve failure backtraces (wip) #798

Open
thomcc wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make sure mentat error types preserve failure backtraces (wip) #798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions query-algebrizer/src/clauses/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use clauses::{
};

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
Result,
};

Expand Down Expand Up @@ -80,12 +80,12 @@ impl ValueTypes for FnArg {

&FnArg::Constant(NonIntegerConstant::BigInteger(_)) => {
// Not yet implemented.
bail!(AlgebrizerError::UnsupportedArgument)
bail!(AlgebrizerErrorKind::UnsupportedArgument)
},

// These don't make sense here. TODO: split FnArg into scalar and non-scalar…
&FnArg::Vector(_) |
&FnArg::SrcVar(_) => bail!(AlgebrizerError::UnsupportedArgument),
&FnArg::SrcVar(_) => bail!(AlgebrizerErrorKind::UnsupportedArgument),

// These are all straightforward.
&FnArg::Constant(NonIntegerConstant::Boolean(_)) => ValueTypeSet::of_one(ValueType::Boolean),
Expand Down Expand Up @@ -196,7 +196,7 @@ impl ConjoiningClauses {
FnArg::Variable(in_var) => {
// TODO: technically you could ground an existing variable inside the query….
if !self.input_variables.contains(&in_var) {
bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone()))
bail!(AlgebrizerErrorKind::UnboundVariable((*in_var.0).clone()))
}
match self.bound_value(&in_var) {
// The type is already known if it's a bound variable….
Expand All @@ -205,7 +205,7 @@ impl ConjoiningClauses {
// The variable is present in `:in`, but it hasn't yet been provided.
// This is a restriction we will eventually relax: we don't yet have a way
// to collect variables as part of a computed table or substitution.
bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone()))
bail!(AlgebrizerErrorKind::UnboundVariable((*in_var.0).clone()))
},
}
},
Expand All @@ -215,7 +215,7 @@ impl ConjoiningClauses {

// These don't make sense here.
FnArg::Vector(_) |
FnArg::SrcVar(_) => bail!(AlgebrizerError::InvalidGroundConstant),
FnArg::SrcVar(_) => bail!(AlgebrizerErrorKind::InvalidGroundConstant),

// These are all straightforward.
FnArg::Constant(NonIntegerConstant::Boolean(x)) => {
Expand Down
34 changes: 17 additions & 17 deletions query-algebrizer/src/clauses/fulltext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use clauses::{
};

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
BindingError,
Result,
};
Expand All @@ -53,25 +53,25 @@ impl ConjoiningClauses {
#[allow(unused_variables)]
pub(crate) fn apply_fulltext(&mut self, known: Known, where_fn: WhereFn) -> Result<()> {
if where_fn.args.len() != 3 {
bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3));
bail!(AlgebrizerErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3));
}

if where_fn.binding.is_empty() {
// The binding must introduce at least one bound variable.
bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable));
bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable));
}

if !where_fn.binding.is_valid() {
// The binding must not duplicate bound variables.
bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable));
bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable));
}

// We should have exactly four bindings. Destructure them now.
let bindings = match where_fn.binding {
Binding::BindRel(bindings) => {
let bindings_count = bindings.len();
if bindings_count < 1 || bindings_count > 4 {
bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(),
bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(),
BindingError::InvalidNumberOfBindings {
number: bindings.len(),
expected: 4,
Expand All @@ -82,7 +82,7 @@ impl ConjoiningClauses {
},
Binding::BindScalar(_) |
Binding::BindTuple(_) |
Binding::BindColl(_) => bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)),
Binding::BindColl(_) => bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)),
};
let mut bindings = bindings.into_iter();
let b_entity = bindings.next().unwrap();
Expand All @@ -95,7 +95,7 @@ impl ConjoiningClauses {
// TODO: process source variables.
match args.next().unwrap() {
FnArg::SrcVar(SrcVar::DefaultSrc) => {},
_ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "source variable", 0)),
_ => bail!(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(), "source variable", 0)),
}

let schema = known.schema;
Expand All @@ -115,10 +115,10 @@ impl ConjoiningClauses {
match self.bound_value(&v) {
Some(TypedValue::Ref(entid)) => Some(entid),
Some(tv) => {
bail!(AlgebrizerError::InputTypeDisagreement(v.name().clone(), ValueType::Ref, tv.value_type()))
bail!(AlgebrizerErrorKind::InputTypeDisagreement(v.name().clone(), ValueType::Ref, tv.value_type()))
},
None => {
bail!(AlgebrizerError::UnboundVariable((*v.0).clone()))
bail!(AlgebrizerErrorKind::UnboundVariable((*v.0).clone()))
}
}
},
Expand All @@ -128,10 +128,10 @@ impl ConjoiningClauses {
// An unknown ident, or an entity that isn't present in the store, or isn't a fulltext
// attribute, is likely enough to be a coding error that we choose to bail instead of
// marking the pattern as known-empty.
let a = a.ok_or(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "attribute", 1))?;
let a = a.ok_or(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(), "attribute", 1))?;
let attribute = schema.attribute_for_entid(a)
.cloned()
.ok_or(AlgebrizerError::InvalidArgument(where_fn.operator.clone(),
.ok_or(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(),
"attribute", 1))?;

if !attribute.fulltext {
Expand Down Expand Up @@ -170,18 +170,18 @@ impl ConjoiningClauses {
FnArg::Variable(in_var) => {
match self.bound_value(&in_var) {
Some(t @ TypedValue::String(_)) => Either::Left(t),
Some(_) => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2)),
Some(_) => bail!(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2)),
None => {
// Regardless of whether we'll be providing a string later, or the value
// comes from a column, it must be a string.
if self.known_type(&in_var) != Some(ValueType::String) {
bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2))
bail!(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2))
}

if self.input_variables.contains(&in_var) {
// Sorry, we haven't implemented late binding.
// TODO: implement this.
bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone()))
bail!(AlgebrizerErrorKind::UnboundVariable((*in_var.0).clone()))
} else {
// It must be bound earlier in the query. We already established that
// it must be a string column.
Expand All @@ -190,13 +190,13 @@ impl ConjoiningClauses {
.and_then(|bindings| bindings.get(0).cloned()) {
Either::Right(binding)
} else {
bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone()))
bail!(AlgebrizerErrorKind::UnboundVariable((*in_var.0).clone()))
}
}
},
}
},
_ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2)),
_ => bail!(AlgebrizerErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2)),
};

let qv = match search {
Expand Down Expand Up @@ -245,7 +245,7 @@ impl ConjoiningClauses {

// We do not allow the score to be bound.
if self.value_bindings.contains_key(var) || self.input_variables.contains(var) {
bail!(AlgebrizerError::InvalidBinding(var.name(), BindingError::UnexpectedBinding));
bail!(AlgebrizerErrorKind::InvalidBinding(var.name(), BindingError::UnexpectedBinding));
}

// We bind the value ourselves. This handily takes care of substituting into existing uses.
Expand Down
26 changes: 13 additions & 13 deletions query-algebrizer/src/clauses/ground.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use clauses::{
use clauses::convert::ValueConversion;

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
BindingError,
Result,
};
Expand Down Expand Up @@ -117,19 +117,19 @@ impl ConjoiningClauses {

pub(crate) fn apply_ground(&mut self, known: Known, where_fn: WhereFn) -> Result<()> {
if where_fn.args.len() != 1 {
bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 1));
bail!(AlgebrizerErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 1));
}

let mut args = where_fn.args.into_iter();

if where_fn.binding.is_empty() {
// The binding must introduce at least one bound variable.
bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable));
bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable));
}

if !where_fn.binding.is_valid() {
// The binding must not duplicate bound variables.
bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable));
bail!(AlgebrizerErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable));
}

let schema = known.schema;
Expand All @@ -145,7 +145,7 @@ impl ConjoiningClauses {
// Just the same, but we bind more than one column at a time.
if children.len() != places.len() {
// Number of arguments don't match the number of values. TODO: better error message.
bail!(AlgebrizerError::GroundBindingsMismatch)
bail!(AlgebrizerErrorKind::GroundBindingsMismatch)
}
for (place, arg) in places.into_iter().zip(children.into_iter()) {
self.apply_ground_place(schema, place, arg)? // TODO: short-circuit on impossible.
Expand All @@ -159,7 +159,7 @@ impl ConjoiningClauses {
// are all in a single structure. That makes it substantially simpler!
(Binding::BindColl(var), FnArg::Vector(children)) => {
if children.is_empty() {
bail!(AlgebrizerError::InvalidGroundConstant)
bail!(AlgebrizerErrorKind::InvalidGroundConstant)
}

// Turn a collection of arguments into a Vec of `TypedValue`s of the same type.
Expand All @@ -177,7 +177,7 @@ impl ConjoiningClauses {
if accumulated_types.insert(tv.value_type()) &&
!accumulated_types.is_unit() {
// Values not all of the same type.
Some(Err(AlgebrizerError::InvalidGroundConstant.into()))
Some(Err(AlgebrizerErrorKind::InvalidGroundConstant.into()))
} else {
Some(Ok(tv))
}
Expand Down Expand Up @@ -208,7 +208,7 @@ impl ConjoiningClauses {

(Binding::BindRel(places), FnArg::Vector(rows)) => {
if rows.is_empty() {
bail!(AlgebrizerError::InvalidGroundConstant)
bail!(AlgebrizerErrorKind::InvalidGroundConstant)
}

// Grab the known types to which these args must conform, and track
Expand All @@ -229,7 +229,7 @@ impl ConjoiningClauses {

if expected_width == 0 {
// They can't all be placeholders.
bail!(AlgebrizerError::InvalidGroundConstant)
bail!(AlgebrizerErrorKind::InvalidGroundConstant)
}

// Accumulate values into `matrix` and types into `a_t_f_c`.
Expand All @@ -245,7 +245,7 @@ impl ConjoiningClauses {
FnArg::Vector(cols) => {
// Make sure that every row is the same length.
if cols.len() != full_width {
bail!(AlgebrizerError::InvalidGroundConstant)
bail!(AlgebrizerErrorKind::InvalidGroundConstant)
}

// TODO: don't accumulate twice.
Expand Down Expand Up @@ -280,13 +280,13 @@ impl ConjoiningClauses {
let inserted = acc.insert(val.value_type());
if inserted && !acc.is_unit() {
// Heterogeneous types.
bail!(AlgebrizerError::InvalidGroundConstant)
bail!(AlgebrizerErrorKind::InvalidGroundConstant)
}
matrix.push(val);
}

},
_ => bail!(AlgebrizerError::InvalidGroundConstant),
_ => bail!(AlgebrizerErrorKind::InvalidGroundConstant),
}
}

Expand All @@ -312,7 +312,7 @@ impl ConjoiningClauses {
self.collect_named_bindings(schema, names, types, matrix);
Ok(())
},
(_, _) => bail!(AlgebrizerError::InvalidGroundConstant),
(_, _) => bail!(AlgebrizerErrorKind::InvalidGroundConstant),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions query-algebrizer/src/clauses/inputs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use mentat_query::{
};

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
Result,
};

Expand Down Expand Up @@ -72,7 +72,7 @@ impl QueryInputs {
let old = types.insert(var.clone(), t);
if let Some(old) = old {
if old != t {
bail!(AlgebrizerError::InputTypeDisagreement(var.name(), old, t));
bail!(AlgebrizerErrorKind::InputTypeDisagreement(var.name(), old, t));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions query-algebrizer/src/clauses/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use mentat_query::{
};

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
Result,
};

Expand Down Expand Up @@ -1013,7 +1013,7 @@ impl ConjoiningClauses {

let qa = self.extracted_types
.get(&var)
.ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()))?;
.ok_or_else(|| AlgebrizerErrorKind::UnboundVariable(var.name()))?;
self.wheres.add_intersection(ColumnConstraint::HasTypes {
value: qa.0.clone(),
value_types: types,
Expand Down
10 changes: 5 additions & 5 deletions query-algebrizer/src/clauses/not.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use mentat_query::{
use clauses::ConjoiningClauses;

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
Result,
};

Expand Down Expand Up @@ -45,7 +45,7 @@ impl ConjoiningClauses {
let col = self.column_bindings.get(&v).unwrap()[0].clone();
template.column_bindings.insert(v.clone(), vec![col]);
} else {
bail!(AlgebrizerError::UnboundVariable(v.name()));
bail!(AlgebrizerErrorKind::UnboundVariable(v.name()));
}
}

Expand Down Expand Up @@ -111,7 +111,7 @@ mod testing {
};

use errors::{
AlgebrizerError,
AlgebrizerErrorKind,
};

use types::{
Expand Down Expand Up @@ -553,8 +553,8 @@ mod testing {
:where (not [?x :foo/knows ?y])]"#;
let parsed = parse_find_string(query).expect("parse failed");
let err = algebrize(known, parsed).expect_err("algebrization should have failed");
match err {
AlgebrizerError::UnboundVariable(var) => { assert_eq!(var, PlainSymbol("?x".to_string())); },
match err.kind() {
&AlgebrizerErrorKind::UnboundVariable(ref var) => { assert_eq!(var, &PlainSymbol("?x".to_string())); },
x => panic!("expected Unbound Variable error, got {:?}", x),
}
}
Expand Down
Loading