Skip to content

Commit

Permalink
remove reference from statically_required_property's FieldValue
Browse files Browse the repository at this point in the history
  • Loading branch information
u9g committed Aug 29, 2023
1 parent 13e2971 commit 734020d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 47 deletions.
13 changes: 0 additions & 13 deletions trustfall_core/src/interpreter/hints/candidates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,6 @@ pub enum CandidateValue<T> {
All,
}

impl<T> CandidateValue<T> {
/// Converts from `&CandidateValue<T>` to `CandidateValue<&T>`.
pub fn as_ref(&self) -> CandidateValue<&T> {
match self {
CandidateValue::Impossible => CandidateValue::Impossible,
CandidateValue::Single(s) => CandidateValue::Single(s),
CandidateValue::Multiple(mult) => CandidateValue::Multiple(mult.iter().collect()),
CandidateValue::Range(r) => CandidateValue::Range(r.as_ref()),
CandidateValue::All => CandidateValue::All,
}
}
}

impl<T: Clone> CandidateValue<&T> {
/// Converts from `CandidateValue<&T>` to `CandidateValue<T>`.
pub fn cloned(&self) -> CandidateValue<T> {
Expand Down
34 changes: 17 additions & 17 deletions trustfall_core/src/interpreter/hints/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ mod static_property_values {

assert_eq!(None, info.statically_required_property("name"));
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(3))),
Some(CandidateValue::Single(FieldValue::Int64(3))),
info.statically_required_property("value"),
);
})),
Expand Down Expand Up @@ -642,7 +642,7 @@ mod static_property_values {

assert_eq!(None, info.statically_required_property("value"));
assert_eq!(
Some(CandidateValue::Single(&FieldValue::String("Prime".into()))),
Some(CandidateValue::Single(FieldValue::String("Prime".into()))),
info.statically_required_property("__typename"),
);
})),
Expand All @@ -667,8 +667,8 @@ mod static_property_values {

assert_eq!(
Some(CandidateValue::Multiple(vec![
&"fourteen".into(),
&"fifteen".into(),
"fourteen".into(),
"fifteen".into(),
])),
info.statically_required_property("name"),
);
Expand All @@ -693,7 +693,7 @@ mod static_property_values {
assert_eq!(vid(1), info.vid());

assert_eq!(
Some(CandidateValue::Range(Range::with_end(Bound::Excluded(&FieldValue::Int64(9)), true))),
Some(CandidateValue::Range(Range::with_end(Bound::Excluded(FieldValue::Int64(9)), true))),
info.statically_required_property("value"),
);
})),
Expand All @@ -716,7 +716,7 @@ mod static_property_values {
assert_eq!(vid(1), info.vid());

assert_eq!(
Some(CandidateValue::Range(Range::with_end(Bound::Included(&FieldValue::Int64(8)), true))),
Some(CandidateValue::Range(Range::with_end(Bound::Included(FieldValue::Int64(8)), true))),
info.statically_required_property("value"),
);
})),
Expand All @@ -743,7 +743,7 @@ mod static_property_values {

assert_eq!(vid(2), neighbor.vid());
assert_eq!(
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(&FieldValue::Int64(25)), true))),
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(FieldValue::Int64(25)), true))),
neighbor.statically_required_property("value"),
);
})),
Expand All @@ -758,7 +758,7 @@ mod static_property_values {
let neighbor = info.destination();
assert_eq!(vid(2), neighbor.vid());
assert_eq!(
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(&FieldValue::Int64(25)), true))),
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(FieldValue::Int64(25)), true))),
neighbor.statically_required_property("value"),
);
}))
Expand Down Expand Up @@ -786,7 +786,7 @@ mod static_property_values {

assert_eq!(vid(2), neighbor.vid());
assert_eq!(
Some(CandidateValue::Range(Range::with_start(Bound::Included(&FieldValue::Int64(24)), true))),
Some(CandidateValue::Range(Range::with_start(Bound::Included(FieldValue::Int64(24)), true))),
neighbor.statically_required_property("value"),
);
})),
Expand All @@ -801,7 +801,7 @@ mod static_property_values {
let neighbor = info.destination();
assert_eq!(vid(2), neighbor.vid());
assert_eq!(
Some(CandidateValue::Range(Range::with_start(Bound::Included(&FieldValue::Int64(24)), true))),
Some(CandidateValue::Range(Range::with_start(Bound::Included(FieldValue::Int64(24)), true))),
neighbor.statically_required_property("value"),
);
}))
Expand Down Expand Up @@ -834,7 +834,7 @@ mod static_property_values {
// are the starting vertex and the neighbors, so the filter is binding
// to all neighboring vertices.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(6))),
Some(CandidateValue::Single(FieldValue::Int64(6))),
neighbor.statically_required_property("value"),
);
})),
Expand All @@ -851,7 +851,7 @@ mod static_property_values {
// are the starting vertex and the neighbors, so the filter is binding
// to all neighboring vertices.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(6))),
Some(CandidateValue::Single(FieldValue::Int64(6))),
destination.statically_required_property("value"),
);
})),
Expand Down Expand Up @@ -1081,7 +1081,7 @@ mod static_property_values {
// Here the value *is* statically known, since the `@optional`
// has already been resolved in a prior step.
assert_eq!(
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(&FieldValue::Int64(1)), true)),),
Some(CandidateValue::Range(Range::with_start(Bound::Excluded(FieldValue::Int64(1)), true)),),
destination.statically_required_property("value"),
);
})),
Expand Down Expand Up @@ -1146,7 +1146,7 @@ mod static_property_values {
// Here the value *is* statically known, since the property value can
// affect which vertices this edge is resolved to.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(1))),
Some(CandidateValue::Single(FieldValue::Int64(1))),
destination.statically_required_property("value"),
);
})),
Expand Down Expand Up @@ -1182,7 +1182,7 @@ mod static_property_values {
// ensures that at least one such value must exist, or else vertices
// from the currently-resolved edge will be discarded.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(1))),
Some(CandidateValue::Single(FieldValue::Int64(1))),
next_neighbor.statically_required_property("value"),
);
})),
Expand All @@ -1201,7 +1201,7 @@ mod static_property_values {
// ensures that at least one such value must exist, or else vertices
// from the currently-resolved edge will be discarded.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(1))),
Some(CandidateValue::Single(FieldValue::Int64(1))),
next_neighbor.statically_required_property("value"),
);
})),
Expand All @@ -1213,7 +1213,7 @@ mod static_property_values {
// Here the value is also statically known, since the property is local
// to the edge being resolved.
assert_eq!(
Some(CandidateValue::Single(&FieldValue::Int64(1))),
Some(CandidateValue::Single(FieldValue::Int64(1))),
destination.statically_required_property("value"),
);
})),
Expand Down
26 changes: 12 additions & 14 deletions trustfall_core/src/interpreter/hints/vertex_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub trait VertexInfo: super::sealed::__Sealed {
/// In contrast, filters relying the value of a `@tag` do not produce
/// statically-required values, since the `@tag` value must be computed at runtime.
/// For this case, see the [`VertexInfo::dynamically_required_property()`] method.
fn statically_required_property(&self, name: &str) -> Option<CandidateValue<&FieldValue>>;
fn statically_required_property(&self, name: &str) -> Option<CandidateValue<FieldValue>>;

/// Check whether the query demands this vertex property to have specific values:
/// a single value, or one of a set or range of values. The candidate values
Expand Down Expand Up @@ -195,7 +195,7 @@ impl<T: InternalVertexInfo + super::sealed::__Sealed> VertexInfo for T {
Box::new(properties.filter(move |r| seen_property.insert(r.name.clone())))
}

fn statically_required_property(&self, property: &str) -> Option<CandidateValue<&FieldValue>> {
fn statically_required_property(&self, property: &str) -> Option<CandidateValue<FieldValue>> {
if self.non_binding_filters() {
// This `VertexInfo` is in a place where the filters applied to fields
// don't actually constrain their value in the usual way that lends itself
Expand Down Expand Up @@ -225,10 +225,11 @@ impl<T: InternalVertexInfo + super::sealed::__Sealed> VertexInfo for T {
let field = relevant_filters.peek()?.left();

let candidate =
compute_statically_known_candidate(field, relevant_filters, query_variables);
compute_statically_known_candidate(field, relevant_filters, query_variables)
.map(|x| x.cloned());
debug_assert!(
// Ensure we never return a range variant with a completely unrestricted range.
candidate.as_ref().unwrap_or(&CandidateValue::All) != &CandidateValue::Range(Range::full()),
candidate.clone().unwrap_or(CandidateValue::All) != CandidateValue::Range(Range::full()),
"caught returning a range variant with a completely unrestricted range; it should have been CandidateValue::All instead"
);

Expand Down Expand Up @@ -283,16 +284,13 @@ impl<T: InternalVertexInfo + super::sealed::__Sealed> VertexInfo for T {
// Early-return in case there are no filters that apply here.
let first_filter = relevant_filters.first()?;

let initial_candidate = self
.statically_required_property(property)
.unwrap_or_else(|| {
if first_filter.left().field_type.nullable {
CandidateValue::All
} else {
CandidateValue::Range(Range::full_non_null())
}
})
.cloned();
let initial_candidate = self.statically_required_property(property).unwrap_or_else(|| {
if first_filter.left().field_type.nullable {
CandidateValue::All
} else {
CandidateValue::Range(Range::full_non_null())
}
});

// Right now, this API only supports materializing the constraint from a single tag.
// Choose which @filter to choose as the one providing the value.
Expand Down
5 changes: 2 additions & 3 deletions trustfall_core/src/schema/adapter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ impl<'a> crate::interpreter::Adapter<'a> for SchemaAdapter<'a> {
match edge_name.as_ref() {
"VertexType" => {
let name = resolve_info.statically_required_property("name");
vertex_type_iter(self.schema, name.map(|x| x.cloned()))
vertex_type_iter(self.schema, name)
}
"Entrypoint" => entrypoints_iter(self.schema),
"Schema" => Box::new(std::iter::once(SchemaVertex::Schema)),
Expand Down Expand Up @@ -427,8 +427,7 @@ impl<'a> crate::interpreter::Adapter<'a> for SchemaAdapter<'a> {
let destination = resolve_info.destination();

// `.cloned()` to get rid of reference, so we can own it when we need to move it later
let vertex_type_name =
destination.statically_required_property("name").map(|x| x.cloned());
let vertex_type_name = destination.statically_required_property("name");

resolve_neighbors_with(contexts, move |_| {
// `.clone()` each time as we may have multiple "vertex_type" edges
Expand Down

0 comments on commit 734020d

Please sign in to comment.